Roman Numeral to Integer - I keep getting the incorrect sum for "MCMXCIV" - Code works fine for "III" and "LVIII"

Question

This is the Javascript code with the mistake:

/**
 * @param {string} s
 * @return {number}
 */

var romanToInt = function(s) {
        //I=1,V=5,X=10,L=50,C=100,D=500,M=1000    
        var arrayChar = s.split(""); //Array to store every individual Roman numeral character
        var count = 0; //Roman numeral summation
        for(var i=0;i<arrayChar.length;i++){//Iterate through each character in the array
           if(i<(arrayChar.length-1)&&(arrayChar[i]===("I"||"X"||"C"))){//If value matches I, X, or C and i is less than arraylength minus 1
                //I before V or X = 4 or 9 respectively
                if(arrayChar[i]==="I"){
                    if(arrayChar[i+1]==="V"){
                        count+=4;
                        i++;
                        console.log(count);
                    }
                    else if(arrayChar[i+1]==="X"){
                        count+=9;
                        i++;
                        console.log(count);
                    }
                    else{
                        count+=1;
                        console.log(count);
                    }
                }

                //X before L or C = 40 or 90 respectively
                if(arrayChar[i]==="X"){
                    if(arrayChar[i+1]==="L"){
                        count+=40;
                        i++;
                        console.log(count);
                    }
                    else if(arrayChar[i+1]==="C"){
                        count+=90;
                        i++;
                        console.log(count);
                    }
                    else{
                        count+=10;
                        console.log(count);
                    }
                }

                //C before D or M = 400 or 900 respectively
                if(arrayChar[i]==="C"){
                    if(arrayChar[i+1]==="D"){
                        count+=400;
                        i++;
                        console.log(count);
                    }
                    else if(arrayChar[i+1]==="M"){
                        count+=900;
                        i++;
                        console.log(count);
                    }
                    else{
                        count+=100;
                        console.log(count);
                    }
                }
            }//End of if
            else{
                if(arrayChar[i]==="I"){
                    count+=1;
                    console.log(count);
                }
                if(arrayChar[i]==="V"){
                    count+=5;
                    console.log(count);
                }
                if(arrayChar[i]==="X"){
                    count+=10;
                    console.log(count+"not else if");
                }
                if(arrayChar[i]==="L"){
                    count+=50;
                    console.log(count);
                }
                if(arrayChar[i]==="C"){
                    count+=100;
                    console.log(count+"not else if");
                }
                if(arrayChar[i]==="D"){
                    count+=500;
                    console.log(count);
                }
                if(arrayChar[i]==="M"){
                    count+=1000;
                    console.log(count+"not else if");
                }
            }//End of else
        }//End of for
    return count;
    };//End of anonymous function

I expected to get "1994" for roman number "MCMXCIV", but instead I get 2214 as the result. For some reason, anything involving the letter "C" it is not process correctly.

This is the output on console:

stdout:
1000not else if
1100not else if
2100not else if
2110not else if
2210not else if
2214

Does anyone see something wrong with my code? I would really appreciate some help with this.

Answer 1

2 things in your code are problematic

First, this doesn't work

arrayChar[i]===("I"||"X"||"C")

Here is how this is interpreted :

• arrayChar[i] is C
• ("I"||"X"||"C") returns the first non-false value as described here, so "I" in that case
• Then it does the equal check, so "C" === "I" in that case, and of course that's false

You can fix that by replacing this by either a regular expression or an array with includes() method, as you wish. So this

arrayChar[i]===("I"||"X"||"C")

Becomes this

["I", "X", "C"].includes(arrayChar[i])

Or this

arrayChar[i].match(/[IXC]/)

After that, if you try, you'll see that it's not completely working. The second issue is caused by your cascading if statements : if arrayChar[i] is "X" and arrayChar[i+1] is "C", it adds 90 then increments i and continues checking subsequent if statements (with one being arrayChar[i]==="C", which is true because of the i++, so it does an unwanted +100). By replacing those if by if and else if, it will fix the second issue.

If you just want the working code, or if my instructions were unclear, feel free to look at this :

   /**
     * @param {string} s
     * @return {number}
     */
    
    var romanToInt = function(s) {
            //I=1,V=5,X=10,L=50,C=100,D=500,M=1000    
            var arrayChar = s.split(""); //Array to store every individual Roman numeral character
            var count = 0; //Roman numeral summation
            for(var i=0;i<arrayChar.length;i++){//Iterate through each character in the array
               if(i<(arrayChar.length-1)&&(arrayChar[i].match(/[IXC]/))){//If value matches I, X, or C and i is less than arraylength minus 1
                    //I before V or X = 4 or 9 respectively
                    if(arrayChar[i]==="I"){
                        if(arrayChar[i+1]==="V"){
                            count+=4;
                            i++;
                            console.log(count);
                        }
                        else if(arrayChar[i+1]==="X"){
                            count+=9;
                            i++;
                            console.log(count);
                        }
                        else{
                            count+=1;
                            console.log(count);
                        }
                    }
    
                    //X before L or C = 40 or 90 respectively
                    else if(arrayChar[i]==="X"){
                        if(arrayChar[i+1]==="L"){
                            count+=40;
                            i++;
                            console.log(count);
                        }
                        else if(arrayChar[i+1]==="C"){
                            count+=90;
                            i++;
                            console.log(count);
                        }
                        else{
                            count+=10;
                            console.log(count);
                        }
                    }
    
                    //C before D or M = 400 or 900 respectively
                    else if(arrayChar[i]==="C"){
                        if(arrayChar[i+1]==="D"){
                            count+=400;
                            i++;
                            console.log(count);
                        }
                        else if(arrayChar[i+1]==="M"){
                            count+=900;
                            i++;
                            console.log(count);
                        }
                        else{
                            count+=100;
                            console.log(count);
                        }
                    }
                }//End of if
                else{
                    if(arrayChar[i]==="I"){
                        count+=1;
                        console.log(count);
                    }
                    else if(arrayChar[i]==="V"){
                        count+=5;
                        console.log(count);
                    }
                    else if(arrayChar[i]==="X"){
                        count+=10;
                        console.log(count+"not else if");
                    }
                    else if(arrayChar[i]==="L"){
                        count+=50;
                        console.log(count);
                    }
                    else if(arrayChar[i]==="C"){
                        count+=100;
                        console.log(count+"not else if");
                    }
                    else if(arrayChar[i]==="D"){
                        count+=500;
                        console.log(count);
                    }
                    else if(arrayChar[i]==="M"){
                        count+=1000;
                        console.log(count+"not else if");
                    }
                }//End of else
            }//End of for
        return count;
        };//End of anonymous function

Have a good day !