Roman Numeral Converter - Object vs. Array

Hi all, here’s my solution to the fcc JS project Roman Numeral Converter. My solution looks more similar to Solution 3 in the Guide.

My questions is, are there any advantages/disadvantages of implementing the mapping from numbers to symbols using an object vs using an array? Readability, efficiency, etc. Lastly, do you find my algorithm easy to understand? If not, how could I improve?
Thanks!

function convertToRoman(num) {

    //Input validation
    if (num < 1 || num >= 4000) {
        console.log("Sorry. This program can only handle numbers between 1-3999");
        return undefined;
    }

    //Break num up into thousands, hundreds, tens, and ones
    let arr = Array.from(num.toString()), y = arr.length - 1;
    arr = arr.map( (x,j) => parseInt(x) * Math.pow(10,y-j) )
    		 .filter( x => x > 0);		//Skip digits == 0

    //Determine the roman numeral representation for each element in arr
    let objarr = [
        { sym: 'I', range: [1, 4] }, 		// 4 = IV
        { sym: 'V', range: [5, 9] }, 		// 9 = IX
        { sym: 'X', range: [10, 40] }, 		// 40 = XL
        { sym: 'L', range: [50, 90] }, 		// 90 = XC
        { sym: 'C', range: [100, 400] }, 	// 400 = CD
        { sym: 'D', range: [500, 900] }, 	// 900 = CM
        { sym: 'M', range: [1000, 3999] }
    ];

    //Notice the geometric sequences in the ranges above
    //I'll call them: 	seq10 = {1,10,100,1000,...} --> 0 and even indices
    //					seq5  = {5,50,500,...} --> odd indices
    //We need this information later to determine the order in which 
    //roman numerals will be generated

    //Comment: A more mathematical approach to identifying which sequence the
    //range falls into is the following test: Number.isInteger(log_10(range[0]))
    //If true then the object is of type "seq10"


    //Convert array to roman numeral symbols
    let symarr = arr.map((n, ni) => {

        //Find the object with the range that n falls into
        let oi = objarr.findIndex(obj => n >= obj.range[0] && n <= obj.range[1]);
        let obj = objarr[oi];

        //Is n at the end of the range?
        if (n === obj.range[1]) {			//Current obj is of type "seq5"
            return oi % 2 !== 0 ? getSymBefore(oi) + getSymAfter(oi) 
                :
                obj.sym + getSymAfter(oi) 	//type "seq10"
        }
        //Otherwise continue
        else {
            let count;
            if (oi % 2 !== 0) { 			//seq5
                count = (n - obj.range[0]) / objarr[oi - 1].range[0]
                return obj.sym + getSymBefore(oi).repeat(count)
            } else { 						//seq10
                count = n / obj.range[0]
                return obj.sym.repeat(count);
            }
        }

    });	//end symarr mapping

    //Finally concatenate and return the symbols in the array
    return symarr.join('');

    //********************
    //FUNCTION DEFINITIONS
    //********************

    function getSymBefore(index) {    	
        return objarr[index - 1].sym;
    }

    function getSymAfter(index) {
        return objarr[index + 1].sym;
    }


}

console.log(convertToRoman(1907));

//[1,5,10,50,100,500,1000] => ['I','V','X','L','C','D','M']
//Rules:
//Don't repeat a symbol more than 3 times in a row
//Smaller symbol before larger symbol indicates subtraction
//	"		"	 after 		"	"		"		addition

// 1 <= n <= 4 --> I,II,III,[IV]
// 5 <= n <= 9 --> V,VI,VII,VIII,[IX]
// 10 <= n <= 50 --> X,XI,XII,XIII,XIV,XV,XVI,XVII,XVIII,XIX,
//						  XX,XXI,XXII,XXIII,XXIV,XXV,XXVI,XXVII,XXVIII,XXIX,
//						  XXX,...,
//					(40)  [XL],XLI,XLII,XLIII
// 50 <= n < 100 --> L,LI,LI,LIII,...,
//						  LX,LXI,LXII, ...,
//						  LXX,...,
//						  LXXX,...,
//					(90)  [XC],...,XCIX
//...and so on

I’m pretty new still but here are my impressions:

Something I learn from a Exercism mentor is that Number() is preferred to parseInt().

parseInt vs Number , the main issue is that the first is not enough strict and can give you wrong results if you don’t take care! For example:

parseInt('123$'); // -> 123 : gives a number as it is not!
Number('123$');   // -> NaN

parseInt('1e2'); // -> 1 : does not handle scientific notation!
Number('1e2');   // -> 100

parseInt('12,345'); // -> 12 : can let you think that you are correctly handle a number in a culture when the decimal separator is a comma!
Number(12,345');    // -> NaN

Second thought was that you don’t need y value at all if you reverse the arr. Puts the ones first. (I checked the code to make sure it works. :slight_smile:

 let arr = Array.from(num.toString()).reverse();
    arr = arr.map( (x,j) => parseInt(x) * Math.pow(10,j) )
    		 .filter( x => x > 0);

Generally, I found your code impenetrable. I think it would be a cleaner approach to compare against a break point instead of a range.

Just my two cents.

It’s very difficult to follow. This is mainly because the way you’ve set it up means lots of boilerplate code & back and forth to actually find the values you want.

Using ranges seriously complicates things here, as does the fact you’re doing a lot of work to allow you to iterate over the number (instead of iterating over the lookup array)

Using an array that maps characters -> values is likely to be better (both in speed terms and in readability) than the array of characters -> ranges you have set up – eg numerals = [{sym: 'M', val: 1000}, ...etc] (or numerals = [['M', 1000], ....etc], that also works fine, with less typing).

One major issue with numerals – which you’ve managed to solve via complex code – is when you have [eg] IV or IX or XC etc. So instead of the complex logic to manage that, it’s much easier to just include those as specific values. eg numerals = [{ sym: 'M', val: 1000}, {sym: 'CM', val: 900}, {sym: 'D', val: 500}, ...etc]

What you can then do is loop through that map, building up the numerals as you adjust the input number. It should need only the most basic of maths, and only a single operation per loop iteration at that. If you want to attempt it this way I’ll leave it up to you to figure out how that might be done. Note that the array I’ve used in the small examples is in descending order, not ascending.

Thanks for your feedback. I didn’t know that about parseInt.

Hi Dan,

Thanks for your constructive input. I first started approaching this problem with arrays but then thought it might be easier for people to understand my code if I linked symbols and ranges… which I guess ended up being a clusterf*ck. Are arrays generally faster than objects?

With the numerals IV, IX, XC, etc. I wrote it that way with the mindset of generalizing the code for higher and higher numbers. Now I see that’s not even needed because there are only a small finite number of characters in the roman numeral system.

This is my first time hearing the term boilerplate code so I had to look it up. What about my code exactly makes it boilerplate? I feel I do not really repeat lines of code anywhere.