JavaScript Algorithms and Data Structures Projects: Roman Numeral Converter

JavaScript
1

Challenge URL: https://learn.freecodecamp.org/javascript-algorithms-and-data-structures/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][0] ) {
      romanN = romanN + romanMap[i][1].repeat(Math.floor(num / romanMap[i][0]));
      convertToRoman(num % romanMap[i][0]);
      break;
    }
  }
  return romanN;
}

convertToRoman(220);
2

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
3

@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][0] ) {
      romanN = romanN + romanMap[i][1].repeat(Math.floor(currentNum / romanMap[i][0]));
      conversion(currentNum % romanMap[i][0]);
      break;
    }
  }};
  conversion(num);
  return romanN;
}

convertToRoman(3220);
4

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.

5

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;
  }
...
6