convert number into roman numerals

Question

The while loop implemented is becoming a potential infinite loop here.And if I implement it using a decrementing for loop i.e for(var i=arrayLen-1;i>=0;i--) then it works fine.What is the issue here?

function convertToRoman(num) {
var roman = "";

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

var arrayLen = Object.keys(lookupObj).length;

while(num>0){

 for (var i=0 ; i<arrayLen ; i++){

  if(num >= Object.keys(lookupObj)[i]){

    roman = roman + lookupObj[Object.keys(lookupObj)[i]];        
    num = num - Object.keys(lookupObj)[i];
    break;

  }
 }
}    

return roman;

}

convertToRoman(1231);

Hint: see what `console.log(Object.keys(lookupObj))` tells you. (Also, not the problem, but `var keys = Object.keys(lookupObj)` would save having to call `Object.keys()` four times on the same object.) — nnnnnn, Feb 23 '17 at 05:19
Your algorithm is reliant on the order of the items in the array returned by `Object.keys(lookupObj)`. The order isn't what you think. (Which is what my previous hint would've showed you.) — nnnnnn, Feb 23 '17 at 05:22
if i do console.log(Object.keys(lookupObj); then it's returning the values in ascending order although the variables are declared in descending.Why so? @nnnnnn — aayushi, Feb 23 '17 at 05:25

score 1 · Accepted Answer · edited May 23 '17 at 11:46

The problem with your code:

You are starting with the first index

for (var i=0 ; i<arrayLen ; i++)

so?

Object.keys(lookupObj)[i]

This happens and that gives 1

Because objects with numeric keys get sorted in ascending order of the numbers.

Result of: Object.keys(lookupObj)

Object.keys(lookupObj)
// ["1", "4", "5", "9", "10", "40", "50", "90", "100", "400", "500", "900", "1000"]

Object.keys(): The order of keys:

Numeric keys are sorted first (even if other keys are present) in ascending order.
String keys are sorted in the order of their creation.
Lastly, Symbols get sorted in the order of their creation

Had your object been anything like:

var lookupObj = {
  1000:"M",
  Z: 'is the last alphabet',
  Apples: 'keep the doctors away',
  900:"CM",
  500:"D",
  400:"CD",
  100:"C",
  90:"XC",
  50:"L",
  40:"XL",
  10:"X",
  9:"IX",   
  4:"IV",
  5:"V",
  1:"I",
};

The result for this would have been

["1", "4", "5", "9", "10", "40", "50", "90", "100", "400", "500", "900", "1000", "Z", "Apples"]

PS: I haven't used iterables, but I am aware that they too impact key ordering. I'll add a link here.

Other points as mentioned to you in comments:

var arrayLen = Object.keys(lookupObj).length; // [1]

while(num>0){
  for (var i=0 ; i<arrayLen ; i++) {
    if(num >= Object.keys(lookupObj)[i]) { // [2]
      roman = roman + lookupObj[Object.keys(lookupObj)[i]];  // [3]    
      num = num - Object.keys(lookupObj)[i]; // [4]
      break;
    }
  }
}

You are calling the same function 4 times, you could save the value in a variable.

var lookupAsArray = Object.keys(lookupObj)

while(num>0) {
  for (var i=0 ; i< lookupAsArray; i++) { // This line is optimised by browsers
    if(num >= lookupAsArray[i]) {         // so is faster than it seems
      roman = roman + lookupObj[lookupAsArray[i]];        
      num = num - lookupAsArray[i];
      break;
    }
  }
}

convert number into roman numerals

1 Answers1

The problem with your code: