Converting to Roman Numerals in Javascript - Weird bug

Question

function convertToRoman(num) {

  var thisMap = {

  1:[1],
  2:[1, 1],
  3:[1, 1, 1],
  4:[1, 5],
  5:[5],
  6:[5, 1],
  7:[5, 1, 1],
  8:[5, 1, 1, 1],
  9:[1, 10],
  0:[0]

  };

  var numMap = {

  1000:"M",
  500:"D",
  100:"C",
  50:"L",
  10:"X",
  5:"V",
  1:"I"

  };

  numArr = num.toString().split("");

  var thisIndex = 1;

  var tallyArr = [];

  for (var i = numArr.length - 1; i >= 0; i--) {

   tallyArr.unshift(thisMap[numArr[i]]);

  }

  thisIndex = Math.pow(10, tallyArr.length - 1);

  checkArr = [];

  <<<BUG HERE>>> 

  for (var x = 0; x < tallyArr.length; x++) {

    for (var y = 0; y < tallyArr[x].length; y++) {

      tallyArr[x][y] *= thisIndex;

    }

    thisIndex = thisIndex / 10;

  }

  <<</BUG HERE>>>

  var finalArr = [];

  for (var a = 0; a < tallyArr.length; a++) {

    for (var b = 0; b < tallyArr[a].length; b++) {

      finalArr.push(numMap[tallyArr[a][b]]);

    }

  }

  finalAnswer = finalArr.join("");

  return finalAnswer;

}

convertToRoman(88);

So this is my function for converting a number into a Roman Numeral in Javascript. It basically formats every number into the right format using thisMap, then uses thisIndex to multiply by either 1000, 100 or 10, and then compares to numMap to get the correct Roman Numeral.

It seems to work in most of the test cases, except with 44, 99, or 3999.

In these cases, it seems to multiply the numbers by the wrong amount, so 44 becomes XLXL, when it should be XLIV.

I think the bug is between the <<>> tags I've inserted, because that is where the numbers seem to be multiplied wrong.

However, I can't spot the problem.

Thanks.

Check this out: http://blog.stevenlevithan.com/archives/javascript-roman-numeral-converter and this SO http://stackoverflow.com/a/9083076/5324369 Is has a working algorithm — Bjørn Sørensen, Aug 29 '16 at 17:55

score 0 · Answer 1 · answered Mar 01 '20 at 09:20

I tried a different approach.

function convertToRoman(num) {
    let arabicArray = [ 1,   4,    5,   9,   10,   40,  50,   90,  100, 400,  500, 900, 1000, 4000, 5000, 5001]
    let romanArray =  ['I', 'IV', 'V', 'IX', 'X', 'XL', 'L', 'XC', 'C', 'CD', 'D', 'CM', 'M', 'MV', 'V', 'limit of 5000']
    let roman = ""
    loop()

    function loop() {
        for (let i = 0; i < arabicArray.length; i++) {
            if (num < arabicArray[i]) {
                roman += romanArray[i - 1]
                num -= arabicArray[i - 1]
                while (num != 0) {loop()} break;
            }
        }
    }

    return roman
}

console.log(convertToRoman(3))

However this gives you a limit to 5000.

score -1 · Answer 2 · answered Aug 29 '16 at 18:33

Try this: x loop should run through all the length of tallyArr except the last one.

function convertToRoman(num) {

  // ... code ...

  for (var x = 0; x < tallyArr.length - 1; x++) {
    for (var y = 0; y < tallyArr[x].length; y++) {
      tallyArr[x][y] *= thisIndex;
    }
    thisIndex = thisIndex / 10;
  }

  // ... more code ...
}

score -1 · Answer 3 · answered Aug 29 '16 at 18:35

Your solution seems a bit over engineered and overly complicated sometimes simpler is better and what might seem like a clever answer and looking for the overly eloquent solution can trip you up.

function convertToRoman(num) {
  var output = "";

  var numMap = [
    { limit: 1000, value: "M" },
    { limit: 900, value: "CM" },
    { limit: 500, value: "D" },
    { limit: 400, value: "CD" },
    { limit: 100, value: "C" },
    { limit: 90, value: "XC" },
    { limit: 50, value: "L" },
    { limit: 40, value: "XL" },
    { limit: 10, value: "X" },
    { limit: 9, value: "IX" },
    { limit: 5, value: "V" },
    { limit: 4, value: "IV" },
    { limit: 1, value: "I" }
  ];

  for(var index = 0; index < numMap.length; index++) {
      var value = numMap[index].value,
          limit = numMap[index].limit;
      while(num >= limit) {
        output += value;
        num -= limit;
      }
  }

  return output;
}

alert(convertToRoman(1));
alert(convertToRoman(4));
alert(convertToRoman(5));
alert(convertToRoman(9));
alert(convertToRoman(10));
alert(convertToRoman(88));
alert(convertToRoman(2016));

JSFiddle

score -1 · Answer 4 · answered Jun 08 '17 at 12:25

var romanNumber = [
            [1, 'I'], [2, 'II'], [3, 'III'],[4, 'IV'],
            [5, 'V'], [6, 'VI'],[7, 'VII'], [8, 'VIII'],
            [9, 'IX'],[10, 'X']
];


function convertToRoman(number) {
    if (number === 0) {
      return '';
    }
    for (var i = 0; i < romanNumber.length; i++) {
      if (number === romanNumber[i][0]) {
        return romanNumber[i][1];
      }
    }
}
console.log(convertToRoman(1));
console.log(convertToRoman(2));
console.log(convertToRoman(3));
console.log(convertToRoman(4));
console.log(convertToRoman(5));
console.log(convertToRoman(6));
console.log(convertToRoman(7));
console.log(convertToRoman(8));
console.log(convertToRoman(9));
console.log(convertToRoman(10));

Converting to Roman Numerals in Javascript - Weird bug

4 Answers4