My code works but it is quite big!

JavaScript
1

Tell us what’s happening:
Hi. So i solved this challenge and i am proud for it but i am really sad due to the fact that my code is far longer than the suggested solutions. What are your thoughts about this? Would an employer hire me with this kind of logic?
Your code so far


function translate(arr){
let roman;
let romans = {
  units:{
    0:'',  
    1:'I',
    2:'II',
    3:'III',
    4:'IV',
    5:'V',
    6:'VI',
    7:'VII',
    8:'VIII',
    9:'IX'  
  },
  tens:{
    0:'',  
    1:'X',
    2:'XX',
    3:'XXX',
    4:'XL',
    5:'L',
    6:'LX',
    7:'LXX',
    8:'LXXX',
    9:'XC'
  },
  hundrends:{
    0:'',
    1:'C',
    2:'CC',
    3:'CCC',
    4:'CD',
    5:'D',
    6:'DC',
    7:'DCC',
    8:'DCCC',
    9:'CM'
  },
  thousands:{
    0:'',
    1:'M',
    2:'MM',
    3:'MMM',
    4:'MMMM',
    5:'MMMMM',
    6:'MMMMMM',
    7:'MMMMMMM',
    8:'MMMMMMMM',
    9:'MMMMMMMMM'    
  }  
};

if(arr.length==1){
  roman = romans['units'][arr[0]];
}
else if(arr.length==2){
    roman = romans['tens'][arr[0]] + romans['units'][arr[1]];
}else if (arr.length==3){
    roman = romans['hundrends'][arr[0]] + romans['tens'][arr[1]] + romans['units'][arr[2]];
}else if(arr.length==4){
    roman = romans['thousands'][arr[0]] + romans['hundrends'][arr[1]] + romans['tens'][arr[2]] + romans['units'][arr[3]];
}
return roman;
}


function convertToRoman(num){
let thousand,hundrend,ten,unit;
let arr = [];
if(num/1000>=1){
    thousand = Math.floor(num/1000);
    arr.push(thousand);
    num=num%1000;
}
if(num/100>=1){
    hundrend = Math.floor(num/100);
    arr.push(hundrend);
    num=num%100;
    console.log(num);
}else{
    hundrend=0;
    num=num%100;
    arr.push(hundrend);
}
if(num/10>=1){
    ten = Math.floor(num/10);
    arr.push(ten);
    num=num%10;   
}else{
    ten=0;
    num=num%10;
    arr.push(ten);
}

if(num/1>=1){
    unit = num;
    arr.push(unit);
    return translate(arr);
}else{
    unit=0;
    num=0;
    arr.push(num);
    return translate(arr);
}    
    
}

**Your browser information:**

User Agent is: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/89.0.4389.90 Safari/537.36.

Challenge: Roman Numeral Converter

Link to the challenge:

2

Now that it works it’s time to take another look and check if some things can’t be simplified and improved. For example, look for places that seem to doing similar thing, just using different values, or long if/else if/else statements. Those are obvious suspects for trying to either, change repeated code into function, incorporate loop, or both.

3

Bug spotted!
4000 != MMMM.

4

Well you wouldn’t be judged solely on that, but it just shows where you’re at in your journey and how much help you may need.

@sanity is right and it’s called “refactoring code” which happens a lot. First solution isn’t always best. After it works it can always be improved.

Congratulations on being thorough with the thousands. On the job you’d ask questions about it. What is the largest number to convert in the tests? Does that indicate the total scope of the converter? Or do you need to do very large numbers? In the challenge “roman numerals” is a link to a tutorial that includes how to represent numbers greater than MMM. I inspected element and learned how to display it in CSS.

Another approach: value in decimal is determined by the column a digit is in. Count the decimal columns and represent the largest first then continue one column down until you get to the last.

An observation on roman numerals: 1,10,100,1000 can be repeated up to 3 times. The 5,50,500 don’t repeat. To get from 3 to 5 and 8 to 10 put a 1 in front of the 5 or 10 (IV or IX or CM)

So, is counting positions or the number of times a letter repeats shorter or more useful? Might be a worthwhile exercise.

5

this is wrong, but the app needs to go only up to 3999 so you can just delete it

6

Hi. I know, i just wanted the thousands to bet here too! :smiley:

7

please put them in as correct then :smiley:
https://www.integers.co/questions-answers/how-is-4000-written-in-roman-numerals.html

8

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