Feedback Request: Roman Numeral Converter

Tell us what’s happening:
Describe your issue in detail here.

I completed this challenge today and am happy with my result. Would love suggestions on how to improve this or any feedback about the solution.

Initially, I solved the problem using a long switch within a while loop. Since it didn’t take me very long and seemed too basic, I wanted to reduce the lines of code.

Solution
function convertToRoman(num) {
       let conversionArray =[];
       let countdown = num;

       while (countdown > 0) {
        switch (countdown > 0) {
                case countdown >= 1000:
                        conversionArray.push("M");
                        countdown -= 1000;
                        break;
                case countdown >= 900:
                        conversionArray.push("CM");
                        countdown -= 900;
                        break;
                case countdown >= 500:
                        conversionArray.push("D");
                        countdown -= 500;
                        break;
                case countdown >= 400:
                        conversionArray.push("CD");
                        countdown -= 400;
                        break;
                case countdown >= 100:
                        conversionArray.push("C");
                        countdown -= 100;
                        break;
                case countdown >= 90:
                        conversionArray.push("XC");
                        countdown -= 90;
                        break;
                case countdown >= 50:
                        conversionArray.push("L");
                        countdown -= 50;
                        break;
                case countdown >= 40:
                        conversionArray.push("XL");
                        countdown -= 40;
                        break;
                case countdown >= 10:
                        conversionArray.push("X");
                        countdown -= 10;
                        break;
                case countdown >= 9:
                        conversionArray.push("IX");
                        countdown -= 9;
                        break;
                case countdown >= 5:
                        conversionArray.push("V");
                        countdown -= 5;
                        break;
                case countdown >= 4:
                        conversionArray.push("IV");
                        countdown -= 5;
                        break;
                case countdown >= 1:
                        conversionArray.push("I");
                        countdown -= 1;
                        break;
                }}
                
        return conversionArray.join("");
};       

console.log(convertToRoman(5000))

       


I read a non-working/help-request post, from a few years ago, that used an object as a lookup tool.

https://forum.freecodecamp.org/t/roman-numeral-converter-is-not-executing/196428

I went in this direction, using it in my own way. I’m proud of this one, so please knock me down a peg!

Your code so far

Solution

function convertToRoman(num) {
 let coverter = num;
 let numArr = [];
 const romanNumberObj = {1000: "M",900: "CM", 500: "D", 400: "CD", 100: "C", 90: "XC", 50: "L", 40: "XL", 10: "X", 9: "IX", 5: "V", 4: "IV", 1: "I"};
 const numLookup = [1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1];

 for (let i = 0; i < numLookup.length; i++) { // breaks down the number into an array of it's roman numeral components
         if (coverter >= numLookup[i]) {
                 numArr.push(numLookup[i]);
                 coverter -= numLookup[i];
                 i -= 1; //backs up i until the converter is reduced by a factor of 10
         }};

 let romanNum = ""; //initializes a variable to build the roman numeral
 numArr.filter( digit => { // filters through the num array, and adds the corresponding roman numeral for each element
         romanNum += romanNumberObj[digit];
          });
 
 return romanNum;

}

console.log(convertToRoman(3999));

Your browser information:

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

Challenge: Roman Numeral Converter

Link to the challenge:

This is a strange way to use a switch. You shouldn’t do this.

Also, notice how repetitive your code looks. That should be an indication that you can simplify your code to make it more maintainable.

Agreed! See the better solution below. (:

Curious what is strange about the switch though?

The value in switch (THIS VALUE HERE) has absolutely no connection to the switch case THIS CONDITION HERE:. That indicates a misuse of a switch.

1 Like

This is a misuse of a filter. Filters are only for making new arrays from old arrays. You should not use them as an forEach.

Good feedback thank you! Are there unintended consequences using filter in this way? I may not understand the differences between filter and forEach, I’ll do some research.

To be honest I expected the filter to return a new array (romanNum) from the old array (numArr). Originally I had;

let romanNum = numArr.filter( digit => {......

But couldn’t get it to work. Then I changed it to:

 let romanNum = []; 
 numArr.filter( digit => { 
         romanNum += romanNumberObj[digit];
          });

But it returned a string anyway.

Thanks again for your time!

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