Is this an efficient implementation of the Roman Numeral Converter?

Question

Is this an efficient way to implement the Roman Numeral Converter ?

So first, I put all the digits of the parameter number into an array.

Then I loop through this array, and multiply a 10 power based number to the current digit.

I use the resulted number in multiple statements, to compare it with special cases or numbers bigger or smaller than those cases.

For each if statement, when it is true, I concatenate a corresponding roman numeral to the whole.

At the end I return the final Roman Numeral as a string.

function convertToRoman(num) {
  
  var digitArr = []; // 2

  while(num !== 0){
    digitArr.push(num % 10); // 2 
    num = Math.floor(num / 10); // num = 0
  }
  
  
  var len = digitArr.length;
  var romanNumber = "";
  var numb;
  var number;
  

  for(var i = len-1;i >= 0;i--){ // 1 0
    number = Math.pow(10, i) * digitArr[i]; // 3

    if(number >= 1000){
      for(var j = 0;j < digitArr[i];j++)romanNumber += "M";
    }else if(number === 900){
      romanNumber += "CM";
    }else if(number > 500){
      numb = number / 100 - 5;
        romanNumber += "D";
        for(var k = 0;k < numb;k++)romanNumber += "C";
    }else if(number === 500){
      romanNumber += "D";
    }else if(number === 400){
      romanNumber += "CD";
    }else if(number < 400 && number >= 100){
      numb = number / 100;
        for(var l = 0;l < numb;l++)romanNumber += "C";
    }else if(number === 90){
      romanNumber += "XC";
    }else if(number >= 60 && number < 90){
      numb = number / 10 - 5;
      romanNumber += "L";
      for(var z = 0;z < numb;z++)romanNumber += "X";
    }else if(number === 50){
      romanNumber += "L";
    }else if(number === 40){
      romanNumber += "XL";
    }else if(number < 40 && number >= 10){
      numb = number / 10;
      for(var x = 0;x < numb;x++)romanNumber += "X";
    }else if(number === 9){
      romanNumber += "IX";
    }else if(number < 9 && number >= 6){
      numb = number - 5;
      romanNumber += "V";
      for(var y = 0;y < numb;y++)romanNumber += "I";
    }else if(number === 5){
      romanNumber += "V";
    }else if(number === 4){
      romanNumber += "IV";
    }else if(number < 4){
      for(var p = 0;p < number;p++){
        romanNumber += "I";
      }
    }
  }

  return romanNumber;
}

Answer 1

See link above in comments for some more elegant solutions. So I am not going to focus on algorithm, which I think can certainly be improved.

Some thoughts on your code itself:

• Comments don't seem to make sense or add value.
• No need for all the if-else stuff. Would be much clearer with just if conditions and breaks, IMO.
• I don't understand why you build digitArray the way you do. How about something like num.toString().split('').map(function(t){return parseInt(t)}) or Array.from(num.toString()).map(Number) (ES6)?
• Your spacing around conditional and flow-control structures (or really, lack thereof) makes your code hard to read.
• Indentation is inconsistent.
• Seems to be odd use of vertical whitespace.