Mistake in this javascript algorithm of roman to integers?

Question

I apologise in advance as the probablility that I have done something stupid is 1 as it's just been a few minutes since I have started problem solving with coding. That being said, please don't mind, I know I'm dumb.

Problem:

Roman numerals are represented by seven different symbols: I, V, X, L, C, D and M.

(Symbol : Value) = (I : 1) (V : 5) (X : 10) (L : 50) (C : 100) (D : 500) (M : 1000)

For example, 2 is written as II in Roman numeral, just two one's added together. 12 is written as XII, which is simply X + II. The number 27 is written as XXVII, which is XX + V + II.

Roman numerals are usually written largest to smallest from left to right. However, the numeral for four is not IIII. Instead, the number four is written as IV. Because the one is before the five we subtract it making four. The same principle applies to the number nine, which is written as IX. There are six instances where subtraction is used:

I can be placed before V (5) and X (10) to make 4 and 9. X can be placed before L (50) and C (100) to make 40 and 90. C can be placed before D (500) and M (1000) to make 400 and 900. Given a roman numeral, convert it to an integer.

Example 1:

Input: s = "III"
Output: 3
Explanation: III = 3.
Example 2:

Input: s = "LVIII"
Output: 58
Explanation: L = 50, V= 5, III = 3.
Example 3:

Input: s = "MCMXCIV"
Output: 1994
Explanation: M = 1000, CM = 900, XC = 90 and IV = 4.

My solution:

let old;
var romanToInt = function(s) {
    let dict = {I:1,V:5,X:10,L:50,C:100,D:500,M:1000};
    let x = 0;
    for(let letter of s){
        x = dict[letter] + x;
        if(old == "I" && letter == "V" || old == "I" && letter == "x"){x = x - 2*old;};
        if(old == "X" && letter == "L" || old == "X" && letter == "C"){x = x - 2*old;};
        if(old == "C" && letter == "D" || old == "C" && letter == "M"){x = x - 2*old;};
        old = letter;
    };
    return x;
};

console.log(romanToInt("VIII"))

Can you please tell why this isn't working ?

Answer 1

In less iteration steps

var romanToInt = function(s) {
    symbolValues = {'I':1,'V':5,'X':10,'L':50,'C':100,'D':500,'M':1000};

    num=0;
    for(let i=0;i<s.length;){
        firstCharValue = symbolValues[s[i]];
        secondCharValue = symbolValues[s[i+1]]??0;

        if(firstCharValue<secondCharValue){
            num+=(secondCharValue-firstCharValue);
            i+=2;
        }else{
            num+=firstCharValue; 
            i+=1;
        }
    }
    return  num;
};

Answer 2

var romanToInt = function(s) {
    let Roman  = {"I": 1, "V":5,"X":10,"L":50,"C":100,"D":500,"M":1000};
    let count = 0;
    
     for (var i = 0; i < s.length; i++) {        
        count += Roman [s[i]];             
    }
    return count;
    
}

Answer 3

Update

Here's something I just learned:

1. The letters I, X, C can be repeated thrice in succession.
2. If a lower value digit is written to the left of a higher value digit, it is subtracted.
3. If a lower value digit is written to the right of a higher value digit, it is added.
4. Only I, X, and C can be used as subtractive numerals.

In the OP there's a ton of flow control statements (ifs) and I believe it was an attempt to satisfy rule 2. and 4.

Solution

Add c: -100, x: -10, and i: -1 key/values to dict. Create 3 RegExp objects -- each pattern covers rule 2. and 4. Before you begin to convert the given string use .replace() to fix all left assigned C, X, I:

...// c, x, i added to dict
        c: -100,
        x: -10,
        i: -1
      };
      const rgxC = new RegExp(/(C)([DM])/, 'g');
      const rgxX = new RegExp(/(X)([LCDM])/, 'g');
      const rgxI = new RegExp(/(I)([VXLCDM])/, 'g');
      roman = roman.replace(rgxC, 'c$2').replace(rgxX, 'x$2').replace(rgxI, 'i$2');

• Example A - Modified OP with solution

• Example B - A terse function fromRoman(string)

• Example C - An inverse function toRoman(number) I barely read the question brain farted and wrote this, DERP! 😩

Example A

const romanToInt = roman => {
  const dict = {
    I: 1,
    V: 5,
    X: 10,
    L: 50,
    C: 100,
    D: 500,
    M: 1000,
    c: -100,
    x: -10,
    i: -1
  };
  const rgxC = new RegExp(/(C)([DM])/, 'g');
  const rgxX = new RegExp(/(X)([LCDM])/, 'g');
  const rgxI = new RegExp(/(I)([VXLCDM])/, 'g');
  roman = roman.replace(rgxC, 'c$2').replace(rgxX, 'x$2').replace(rgxI, 'i$2');
  let sum = 0;
  for (let char of roman) {
    sum = dict[char] + sum;
  }
  return sum;
};

console.log(romanToInt('MLIX'));
console.log(romanToInt('MCDIX'));

Example B

/**
 * Converts a given Roman numeral to a number 
 * @param {String} Roman numeral  to convert
 * @return {Number} a Number
 */
function fromRoman(roman) {
  // Object of Roman numbers and decimal equivalents
  const R = {
    M: 1000,
    D: 500,
    C: 100,
    L: 50,
    X: 10,
    V: 5,
    I: 1,
    i: -1,
    x: -10,
    c: -100
  };
  const rgxC = new RegExp(/(C)([DM])/, 'g');
  const rgxX = new RegExp(/(X)([LCDM])/, 'g');
  const rgxI = new RegExp(/(I)([VXLCDM])/, 'g');
  roman = roman.replace(rgxC, 'c$2').replace(rgxX, 'x$2').replace(rgxI, 'i$2');
  /*
  Split the Roman numeral into an array of characters 
  then use each char as the key of object R to get the 
  number value and add each number and return the sum.
  */
  return roman.split('').reduce((sum, r) => sum + R[r], 0);
}

console.log(fromRoman('MCMLXXXVI'));
console.log(fromRoman('LXCIV'));

Example C

/**
 * Helper function divides 1st param by the second
 * param to get the quotient and remainder
 * @param {Number} dividend to divide into
 * @param {Number} divisor to divide by
 * @return {Array} [0] = quotient [1] = remainder
 */
const div = (dividend, divisor) => 
[Math.floor(dividend / divisor), (dividend % divisor)];

/**
 * Converts a given number to a Roman numeral
 * @param {Number} int an integer to convert
 * @return {String} a Roman numeral
 */
function toRoman(int) {
  // A 2D array of Roman numbers and decimal equivalents
  const R = [
    ['M', 1000],
    ['D', 500],
    ['C', 100],
    ['L', 50],
    ['X', 10],
    ['V', 5],
    ['I', 1]
  ];
  /*
   Returns a 2D array of only destructured sub-arrays that are of 
   lesser value at R[?][1] than the given number. Basically 
   determining what we can divide with and get a whole number
   */
  const maxD = R.filter(([r, n]) => int > n);
  /*
  The outer loop destructures the filtered 2D array on each 
  iteration, the function div() divides the given number by 
  maxD[?][1] returning an array called "qr". "qr" contians the 
  quotient and remainder.
  Then in the inner loop maxD[?][0] is added to the empty array 
  (rArray) as many times as qr[0] (quotient) and qr[1] (remainder)
  becomes the new int to divide into.
  */
  let roman = [];
  for (const [rom, num] of maxD) {
    let qr = div(int, num);
    int = qr[1];
    for (let q = 0; q < qr[0]; q++) {
      roman.push(rom);
    }
  }
  /*
  Convert array into a string and correct any 9s or 4s to 
  satisfy rule 1.
  */
  return roman.join('').replace('VIIII', 'IX').replace('IIII', 'IV');
};

console.log(toRoman(204));
console.log(toRoman(6089));

Answer 4

Would you be up for a different solution? Since the logic of roman numerals follows a consistent pattern with a single look back, this would be much easier to write as a loop with a look back condition rather than a sum of multiple conditions on the string value. Let me explain.

Since all string values are associated with a numeric value, you could just convert every character in the string to an integer and then add them after.

To handle the subtraction (thank you for explaining this I honestly had no idea it worked like this) you could look back after every time you convert a character to a numeral and see if the previous numeral is lower than your current value. This would tell you a subtraction is needed. Below, I solve this by keeping the original lower value, lets say 10, and when the current string character value is greater at 50, I take 50 and subtract the previous value 10 twice. This removes the previous lesser value and then again to create the overall subtraction from 50. (10 + 50 - 10 - 10 = 40).

var data = {I:1,V:5,X:10,L:50,C:100,D:500,M:1000};
var input = 'MCMXCIV'
console.log(
    input.split('').map((v,i,e) => data[e[i-1]] < data[v] ? data[v]-(2*data[e[i-1]]): data[v]).reduce((c,v) => c+v)
)

A commented version to explain the code

input.split('').map( //split the roman numeral string into an array of characters and then "map" over each one
    // v = current roman numeral, i = current index of array, e = all elements of array
    (v,i,e) => 
    data[e[i-1]] < data[v] // if previous roman numeral value is less than the current ... 
        ? data[v]-(2*data[e[i-1]]) // subtract the current value by 2 times the previous lesser value
        : data[v] // convert roman numeral to integer
    ).reduce((c,v) => c+v) // reduce the array of values into a single integer