# JavaScript Algorithms and Data Structures Projects: Roman Numeral Converter

When I run my solution to this problem in my browser’s console, it seems to be generating all of the expected values; however, when I try submitting the code snippet below on FreeCodeCamp, none of the tests are returning the expected result. Do you have any suggestions on where I may be going wrong between my browser console and FCC?

My Solution:

``````let romanN = [];

function convertToRoman(num) {
let romanMap = [
[ 1000, 'M' ],
[ 900, 'CM' ],
[ 500, 'D' ],
[ 400, 'CD' ],
[ 100, 'C' ],
[ 90, 'XC' ],
[ 50, 'L' ],
[ 40, 'XL' ],
[ 10, 'X' ],
[ 9, 'IX' ],
[ 5, 'V' ],
[ 4, 'IV' ],
[ 1, 'I' ]
];

for (let i = 0; i < romanMap.length; i++) {
if ( num >= romanMap[i] ) {
romanN = romanN + romanMap[i].repeat(Math.floor(num / romanMap[i]));
convertToRoman(num % romanMap[i]);
break;
}
}
return romanN;
}

convertToRoman(220);
``````

The problem is the global variable `romanN`. Make it local to the function, `convertToRoman(..)`.

Each function call will add to the `romanN`. Since the test calls the function multiple times, `romanN` will accumulate the result of every function call. Hence, you will get a massive string at the end of the test suite.

Also, your algorithm is incorrect so you will still fail some test after fixing this.

As a side note, if you meant to use string, use string instead of an array.

1 Like

@gunhoo93, Thanks so much for the fast reply! I didn’t realize I couldn’t declare a variable outside of the given function, but it makes perfect sense! I just wrapped my code in a new internal function that’s called recursively on itself (and fixed the silly declaration of romanN as an array!). You mentioned there’s something else wrong with my algorithm; however, it passed the test suite fine. Can you share what is incorrect and the case where it would provide an incorrect value so I may learn?

Here’s the code I submitted and passed the test suite:

``````function convertToRoman(num) {
let romanMap = [
[ 1000, 'M' ],
[ 900, 'CM' ],
[ 500, 'D' ],
[ 400, 'CD' ],
[ 100, 'C' ],
[ 90, 'XC' ],
[ 50, 'L' ],
[ 40, 'XL' ],
[ 10, 'X' ],
[ 9, 'IX' ],
[ 5, 'V' ],
[ 4, 'IV' ],
[ 1, 'I' ]
];

let romanN = '';

function conversion(currentNum) {
for (let i = 0; i < romanMap.length; i++) {
if ( currentNum >= romanMap[i] ) {
romanN = romanN + romanMap[i].repeat(Math.floor(currentNum / romanMap[i]));
conversion(currentNum % romanMap[i]);
break;
}
}};
conversion(num);
return romanN;
}

convertToRoman(3220);
``````

The incorrect algorithm is from the code you originally posted that missed the initial call to `conversion(..)`. Your updated code doesn’t have this problem.

As a side note, there is a much simpler implementation.

I used while loop for everything like this:

`````` var str="";

while(num-1000>=0) {
str+= "M";
num -=1000;
}
while(num-900>=0) {
str+= "CM";
num -=900;
}
while(num-500>=0) {
str+= "D";
num -=500;
}
...
``````