Roman Numeral Converter Optimization

Tell us what’s happening:

Any suggestions on optimizing this? It’s a tedious mess as written.

Your code so far

function convertToRoman(num) {
  
  var ones = num%10;
  var onesaf = ones%10%5;
  var onesf = (ones-ones%5)/5;
  var tens = (num%100-num%10)/10;
  var tensaf = tens%10%5;
  var tensf = (tens-tens%5)/5;
  var hundreds = (num%1000-num%100)/100;
  var hundredsaf = hundreds%10%5;
  var hundredsf = (hundreds-hundreds%5)/5;
  var thousands = (num%10000-num%1000)/1000;
  var thousandsaf = thousands%10%5;
  var thousandsf = (thousands-thousands%5)/5;
  
  romone=[];
  romfive=[];
  romten=[];
  romtenfive=[];
  romhun=[];
  romhunfive=[];
  romthou=[];
  
  for (let i=0; i<onesaf; i++){
    romone.push("I");
  }
  romone = romone.join("");  
  if (onesf == 1) {
    romfive.push("V");
  }  
  if (ones == 4 && onesf == 0) {
    romone = ["IV"];
    romfive = [];
  }
  if (ones == 9) {
    romone = ["IX"];
    romfive = [];
  }
  
  for (let i=0; i<tensaf; i++){
    romten.push("X");
  }
  romten = romten.join("");
  if (tensf == 1) {
    romtenfive.push("L");
  }
  if (tens == 4 && tensf == 0) {
    romten = ["XL"];
    romtenfive = [];
  }
  if (tens == 9) {
    romten = ["XC"];
    romtenfive = [];
  }

  
  for (let i=0; i<hundredsaf; i++){
    romhun.push("C");
  }
  romhun = romhun.join("");
  if (hundredsf == 1) {
    romhunfive.push("D");
  }
  if (hundreds == 4 && hundredsf == 0) {
    romhun = ["CD"];
    romhunfive = [];
  }
  if (hundreds == 9) {
    romhun = ["CM"];
    romhunfive = [];
  }

  
  for (let i=0; i<thousandsaf; i++){
    romthou.push("M");
  }
  romthou = romthou.join("");
  
  return romthou + romhunfive + romhun + romtenfive + romten + romfive + romone;

  
}

convertToRoman(798);

Your browser information:

Your Browser User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.132 Safari/537.36.

Link to the challenge:
https://www.freecodecamp.org/challenges/roman-numeral-converter

@firezdog,
I have a few suggestions regarding refactoring your code:

  • I would change the variable names to represent something more meaningful and informative.

  • Instead of having a list of if cases, use a switch and refactor all the code for it in a separate function.

  • Since you are using several for loops you could create a function which takes the for loop limit, and an argument that represents what needs to be iterated on

Hope that helps

Thanks. I’ll think about how to work those ideas in. In terms of meaningful names, I always feel like the more meaningful the name, the less convenient it is to type. How do you strike the right balance between clarity and concision?

The thing I keep in mind when trying to name variables is to make them as clear as possible to anyone else who is reading my code. Your code should read like a story and be descriptive enough so its intention is easy to understand.

Instead of having several variables for each letter in the roman alphabet, why not create an object with certain fields that correspond to the letters? It would save you all the declarations, keep your code in one object and be more descriptive.

1 Like