0

I know there are a few different ways to Romanise a number, however I'm trying to make up my own and was wondering if this approach is possible.

It returns Null for every iteration and I can't seem to see what I am missing here. Any help would be much appreciated!

var array;
var result = [];

function convertToRoman(num) {
  array = num.toString().split('').reverse();

  roman = ["", "I", "II", "III", "IV", "V", "VI", "VII", "VIII", "IX",
    "", "X", "XX", "XXX", "XL", "L", "LX", "LXX", "LXXX", "XC",
    "", "C", "CC", "CCC", "CD", "D", "DC", "DCC", "DCCC", "CM"
  ];

  for (i = 0, j = 0; i < array.length; i++, j += 10) {
    var location = array[i] + j;
    result = result.concat(roman[location]);
  }

  return result.reverse().join();
}

console.log(convertToRoman(123));
mplungjan
  • 169,008
  • 28
  • 173
  • 236
Augustinas
  • 41
  • 5
  • 1
    Try `j + array[i]` instead. Uh. `array[i]` is a string, to do what you meant to do it's better to use `location = 10*j + Number(array[i])`. – Bergi Jan 21 '17 at 14:30
  • 1
    You really really really should declare (and initialise) `array` and `result` inside of your function. Or else it'll run havoc when you call it for the second time. – Bergi Jan 21 '17 at 14:31
  • 1
    `for (var i = 0, j = 0; i < array.length; i++, j += 10) { var location = array[i] + j; result = result.concat(roman[location]); console.log(location,result) }` will give you some idea – mplungjan Jan 21 '17 at 14:31
  • Possible [http://stackoverflow.com/questions/9083037/convert-a-number-into-a-roman-numeral-in-javascript] (dublicate) – Niku Hysa Jan 21 '17 at 14:36
  • Thank you all for your advice. Indeed the problem was the array containing strings and not actual numbers. Converting the array back into integers with either Number() or parseInt() fixed it. Also thank you for the tips on scope. – Augustinas Jan 21 '17 at 15:16

1 Answers1

2

As array contains strings, you have to parse them back to integers when using them in the arithmetic. This is a possible solution:

var roman = ["", "I", "II", "III", "IV", "V", "VI", "VII", "VIII", "IX",
  "", "X", "XX", "XXX", "XL", "L", "LX", "LXX", "LXXX", "XC",
  "", "C", "CC", "CCC", "CD", "D", "DC", "DCC", "DCCC", "CM"
];

function convertToRoman(num) {
  var result = [];
  var array = num.toString().split('').reverse();

  for (i = 0, j = 0; i < array.length; i++, j += 10) {
    var location = parseInt(array[i]) + j;
    result.push(roman[location]);
  }

  return result.reverse().join('');
}

console.log(convertToRoman(123));

Note that I made some changes regarding variable scope and use of array.push(). I also didn't try the method for multiple inputs. But getting the algorithm correct was not part of your question

Michael Ritter
  • 580
  • 2
  • 9