Convert numbers to roman numerals using for loop

Tell us what’s happening:
Describe your issue in detail here.
I’d like some feedback in my solution to make it more efficient using for loop or to improve my code.

   **Your code so far**

function convertToRoman(num) {
 //values of the numbers
 let numerals = {
   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,
 };
 //passing the new values of the new numbers to convert into numerals
 let newNumeral = "";
let numCopy = num;   //copy the num so it doesn't mutate and can used inside the for loop

 //checking the values of numerals objects
 for (let i in numerals) {
 //using the j variable of num, if the num is still greater than the numerals index 
//it will increment the key to the variable newNumerals and stop the loop,
// it will subtract the num to the values of numerals .
   for (let j= 0; j <= num; j++ ){
     if(numCopy >= numerals[i]) {
       newNumeral += i;
       numCopy -= numerals[i];
     }  
   }
 }
 //return to newNumerals to see the new value.
return newNumeral;
}

console.log(convertToRoman(8));
   **Your browser information:**

User Agent is: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/98.0.4758.109 Safari/537.36

Challenge: Roman Numeral Converter

Link to the challenge:

You can tidy it up by using functional methods, but that’s essentially how I approached the problem too. I think it’s a very clean approach.

2 Likes

Thank you! I’ll try to do it by using functional methods! :slight_smile:

2 Likes

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