Roman Numeral Converter Solution - Efficient or Not?

This is my solution to the Roman Numeral Converter project. Is this an efficient way to solve the challenge? I couldn’t really think of any other way to do it. Any suggestions to improve the code would be greatly appreciated. Thanks!

function convertToRoman(num) {
const rNumerals = [1, "I", 4, "IV", 5, "V", 9, "IX", 10, "X", 40, "XL", 50, "L", 90, "XC", 100, "C", 400, "CD", 500, "D", 900, "CM", 1000, "M", 2000, "MM", 3000, "MMM", 4000, "MMMM", 5000, "V̅"];
const rNumeralsTens = [10, "X", 20, "XX", 30, "XXX", 40, "XL", 50, "L", 60, "LX", 70, "LXX", 80, "LXXX", 90, "XC"]
const romanNumeral = [];
 let v;

while (num > 1) {
if (rNumerals.includes(num)) {
romanNumeral.push(rNumerals[rNumerals.indexOf(num)+1]);
break;
}
for (let i = 0; i < rNumerals.length; i++) {
if (rNumerals[i] < num || isNaN(rNumerals[i])) {}
else {v = rNumerals[i-2];
while (num > v) {
  num = num-v;
  romanNumeral.push(rNumerals[i-1]);
}
}
}
if (rNumeralsTens.includes(num)) {
romanNumeral.push(rNumeralsTens[rNumeralsTens.indexOf(num)+1]);
break;
}
if (num === 4) {
romanNumeral.push("IV");
num = 0;
};
if (num === 5) {
romanNumeral.push("V");
num = 0; 
}
if (num === 9) {
romanNumeral.push("IX");
num = 0; 
}
}
if (num === 3 || num === 2 || num === 1) {
while (num > 0) {
  num--;
  romanNumeral.push("I")
}
}
return romanNumeral.join("")
}
convertToRoman(230);
  **Your browser information:**

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

Challenge: JavaScript Algorithms and Data Structures Projects - Roman Numeral Converter

Link to the challenge:

You can write this long arrays like this for example:

const rNumerals = [
1, "I", 
4, "IV", 
5, "V", 
9, "IX", 
10, "X", 
40, "XL", 
50, "L", 
90, "XC", 
100, "C", 
400, "CD", 
500, "D", 
900, "CM", 
1000, "M", 
2000, "MM", 
3000, "MMM", 
4000, "MMMM", 
5000, "V̅"
];

Using objects for this type of stuff instead of arrays also makes sense.

1 Like

Thanks @admit8490 , that makes it a little more readable that way.

Formatting:

function convertToRoman(num) {
  const rNumerals = [1, "I", 4, "IV", 5, "V", 9, "IX", 10, "X", 40, "XL", 50, "L", 90, "XC", 100, "C", 400, "CD", 500, "D", 900, "CM", 1000, "M", 2000, "MM", 3000, "MMM", 4000, "MMMM", 5000, "V̅"];
  const rNumeralsTens = [10, "X", 20, "XX", 30, "XXX", 40, "XL", 50, "L", 60, "LX", 70, "LXX", 80, "LXXX", 90, "XC"]
  const romanNumeral = [];
  let v;

  while (num > 1) {
    if (rNumerals.includes(num)) {
      romanNumeral.push(rNumerals[rNumerals.indexOf(num) + 1]);
      break;
    }
    for (let i = 0; i < rNumerals.length; i++) {
      if (rNumerals[i] < num || isNaN(rNumerals[i])) { }
      else {
        v = rNumerals[i - 2];
        while (num > v) {
          num = num - v;
          romanNumeral.push(rNumerals[i - 1]);
        }
      }
    }
    if (rNumeralsTens.includes(num)) {
      romanNumeral.push(rNumeralsTens[rNumeralsTens.indexOf(num) + 1]);
      break;
    }
    if (num === 4) {
      romanNumeral.push("IV");
      num = 0;
    };
    if (num === 5) {
      romanNumeral.push("V");
      num = 0;
    }
    if (num === 9) {
      romanNumeral.push("IX");
      num = 0;
    }
  }
  if (num === 3 || num === 2 || num === 1) {
    while (num > 0) {
      num--;
      romanNumeral.push("I")
    }
  }
  return romanNumeral.join("")
}
convertToRoman(230);

You have a lot of special case logic that does basically the same thing. I would look at combing similar code.

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