CODEWITHSUNDEEP
javascriptalgorithmfor-looproman-numerals
aayushi
aayushi

Reputation: 359

convert number into roman numerals

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);

Upvotes: 1

Views: 2359

Answers (1)

Amresh Venugopal
Amresh Venugopal

Reputation: 9549

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:

  1. Numeric keys are sorted first (even if other keys are present) in ascending order.
  2. String keys are sorted in the order of their creation.
  3. 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;
    }
  }
}

Upvotes: 1

Related Questions