What do you think of this solution? (Roman Numeral Converter)

I just finished the roman numeral convertor challenge, and I wanted to see if there is any ways to improve. What do y’all think?

function convertToRoman(num) {

  const chart = {
    M:	1000,
    CM:	900,
    D:	500,
    CD:	400,
    C:	100,
    XC:	90,
    L:	50,
    XL:	40,
    X:	10,
    IX:	9,
    V:	5,
    IV:	4,
    I:	1
  };

  let remainder = num;
  const numeralCounts = {};

  // for each numeral type, check how much you can 
  // remove and keep track of that amount
  for (const numeral in chart) {

    // modulo (%) returns the remainder of two values in a 
    // division, for example 3 % 2 = 1 
    // since two goes into three once, leaving 1 left over.
    // this can be used to check if a given numeral can be
    // removed from the number
    let tempRemainder = remainder % chart[numeral];
    if (tempRemainder === remainder) {
      console.log(`Cannot remove ${chart[numeral]} from ${remainder}\n`)
      continue;
    };
    
    // check how many of the numeral you can remove
    let count = Math.trunc(remainder / chart[numeral]);
    remainder = tempRemainder;
    numeralCounts[numeral] = count;
    console.log(`Amount of ${chart[numeral]}: ${numeralCounts[numeral]}\n`)
  }


  // assemble the answer
  let answer = ""
  for (const numeral in numeralCounts) {
    let count = numeralCounts[numeral];
    while (count > 0) {
      answer += numeral;
      count--;
      console.log(answer);
    }
  }
  
  
  return answer;
}

convertToRoman(36);

EDIT: We have blurred this solution (with [spoiler][/spoiler] tags) so that users who have not completed this challenge can read the discussion in this thread without giving away the solution.

:balloon:Hello! Welcome to the forum!

Nice work on completing the challenge. The code definitely works and the beauty of code is that it can always be improved.

The for…in loop will could be improved by limiting the values it processes to only those that are interesting. An interesting value will be something less than or equal to (<=) the value of the remainder variable. If you create a conditional in the for…in loop that first check the numeral value from the chart is greater than the value in remainder, and if so, continues, then you know you will only process the interesting values. In other words, since the value of 36 is passed into convertToRoman, everything in chart above the value of ten is uninteresting (in this case more than 60% of the values in the chart object). So flying over the uninteresting properties of the chart object could make processing a bit quicker.

In this case the chart is very small, but it could be a large json dataset retreived from an internet server, and iterating through that could be very slow indeed.

If you’d like to look at some issues others had with the challenge, click on the ‘Get Help’ button - and yes solutions aren’t available for certification projects but there is often information that can illuminate the proverbial darkness.

Does this help?
Keep up the good progress!

Happy Coding! :slightly_smiling_face:

1 Like

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.