roman to integer leetcode in C fails for MCMXCIV

Question

The last test case of s = "MCMXCIV" is not working. the output is 3099 instead of 1994. I have added all the roman numerals to integers and can't figure out where the issue is. The first two cases were properly executed to give expected output.

// code start
int romanToInt(char *s) {
    int sum = 0;
    if (*s == NULL) {
        return NULL;
    }
    for (int i = 0; i < strlen(s); i++) {
        if (s[i] == 'M') {
            sum = sum + 1000;
        }
        if (s[i] == 'D') {
            sum = sum + 500;
        }
        if (s[i] == 'C') {
            sum = sum + 100;
        }
        if (s[i] == 'L') {
            sum = sum + 50;
        }
        if (s[i] == 'X') {
            sum = sum + 10;
        }
        if (s[i] == 'V') {
            sum = sum + 5;
        }
        if (s[i] == 'I') {
            sum = sum + 1;
        }
        if (i > 0) {
            if (s[i] == 'V' && s[i - 1] == 'I') {
                sum = sum + 3;
            }
            if (s[i] == 'X' && s[i - 1] == 'I') {
                sum = sum + 8;
            }
            if (s[i] == 'L' && s[i - 1] == 'X') {
                sum = sum + 30;
            }
            if (s[i] == 'C' && s[i - 1] == 'X') {
                sum = sum + 80;
            }
            if (s[i] == 'D' && s[i - 1] == 'C') {
                sum = sum + 300;
            }
            if (s[i] == 'M' && s[i - 1] == 'C') {
                sum = sum + 800;
            }
        }
        //sum represents the converted integer
    }
    return sum;
}//code end

Answer 1

There are multiple problems:

• comparing *s to NULL is incorrect: NULL is a macro representing a null pointer, *s is not a pointer, it is a character, which you can compare to '\0' or 0 instead. Your code compiles by chance if NULL is defined as 0 or 0L but would not for the alternative classic definition ((void *)0).

• this initial test is useless anyway as the for loop will stop immediately for an empty string.

• the test i < strlen(s) may recompute the string length at each iteration, which is inefficient. You could just test if s[i] != '\0'.

• the rule for roman numerals is simple: if a letter value is less than that of the next letter, its value is subtracted from the value of the next one.

Here is a modified version:

int romanDigit(char c) {
    switch (c) {
      case 'I': return 1;
      case 'V': return 5;
      case 'X': return 10;
      case 'L': return 50;
      case 'C': return 100;
      case 'D': return 500;
      case 'M': return 1000;
      default: return 0;
    }
}

int romanToInt(const char *s) {
    int sum = 0;
    while (*s) {
        int digit = romanDigit(*s++);
        int next = romanDigit(*s);
        if (digit && digit < next) {
            sum += next - digit;
            s++;
        } else {
            sum += digit;
        }
    }
    return sum;
}

This simplistic implementation works for MCMXCIV (1994) but does not detect invalid numbers such as MXMIV, MVMI, XMMIV... which will be all produce 1994 as well. The classic representations only use I, X and C in front of the next 2 letters, but many variations have been found throughout the long history of Roman Numerals.

Roman numerals embody one of the most durable human inventions of all time: counting. The shape of the letters I, V and X look amazingly similar to Tally marks found on prehistoric artifacts. It gives me the chills to see in the most scientifically advanced publications the very shapes used by stone age cavemen to count.