# JavaScript Algorithms and Data Structures Projects: Roman Numeral Converter Feedback

Hello I have just completed the Roman Numerals Converter JS project and I would really appreciate your feedback on my project.

My code is working correctly for all cases, but after looking at solutions of others I saw much more elegant and efficient-looking solutions than mine, such using recursions or more built-in functions of JavaScript.

For me my code makes perfect sense, while I’m struggling to understand the logic behind some of the posted solutions, but should I be looking into refactoring my code before adding it to my portfolio?

Best regards!

``````function convertToRoman(num) {
// only works up to 9999
let numString = num.toString();
let numSplit = {'thousands' : 0,
'hundreds': 0,
'tens' : 0,
'ones' : 0};
if (numString.length == 1) {
numSplit['ones'] = num;
} else if (numString.length == 2) {
numSplit['ones'] = Number.parseInt(numString[numString.length-1]);
numSplit['tens'] = Number.parseInt(numString[0] + '0');
} else if (numString.length == 3) {
numSplit['ones'] = Number.parseInt(numString[numString.length-1]);
numSplit['tens'] = Number.parseInt(numString[1] + '0');
numSplit['hundreds'] = Number.parseInt(numString[0] + '00');
} else {
numSplit['ones'] = Number.parseInt(numString[numString.length-1]);
numSplit['tens'] = Number.parseInt(numString[2] + '0');
numSplit['hundreds'] = Number.parseInt(numString[1] + '00');
numSplit['thousands'] = Number.parseInt(numString[0] + '000');
}

let romanSymbols = ['I', 'V', 'X', 'L', 'C', 'D', 'M'];
let romanNumOnes = '';
let romanNumTens = "";
let romanNumHundreds = "";
let romanNumThousands = "";

for (let prop in numSplit) {
if (prop == 'ones') {
let currVal = numSplit[prop];

if (currVal < 4) {
for (let i=0; i < currVal; i++) {
romanNumOnes += romanSymbols[0];
}
} else if (currVal == 4) {
romanNumOnes = romanSymbols[0] + romanSymbols[1];
} else if (currVal > 4 && currVal < 9) {
romanNumOnes = romanSymbols[1];
for (let i=5; i < currVal; i++) {
romanNumOnes += romanSymbols[0];
}
} else {
romanNumOnes = romanSymbols[0] + romanSymbols[2];
}
} else if (prop == 'tens') {
let currVal = Number.parseInt(numSplit[prop]
.toString()
.slice(0,1));
if (currVal < 4) {
for (let i=0; i < currVal; i++) {
romanNumTens += romanSymbols[2];
}
} else if (currVal == 4) {
romanNumTens = romanSymbols[2] + romanSymbols[3];
} else if (currVal > 4 && currVal < 9) {
romanNumTens = romanSymbols[3];
for (let i=5; i < currVal; i++) {
romanNumTens += romanSymbols[2];
}
} else {
romanNumTens = romanSymbols[2] + romanSymbols[4];
}
} else if (prop == 'hundreds') {
let currVal = Number.parseInt(numSplit[prop]
.toString()
.slice(0,1));
if (currVal < 4) {
for (let i=0; i < currVal; i++) {
romanNumHundreds += romanSymbols[4];
}
} else if (currVal == 4) {
romanNumHundreds = romanSymbols[4] + romanSymbols[5];
} else if (currVal > 4 && currVal < 9) {
romanNumHundreds = romanSymbols[5];
for (let i=5; i < currVal; i++) {
romanNumHundreds += romanSymbols[4];
}
} else {
romanNumHundreds = romanSymbols[4] + romanSymbols[6];
}
} else if (prop == 'thousands') {
let currVal = Number.parseInt(numSplit[prop]
.toString()
.slice(0,1));
for (let i=0; i < currVal; i++) {
romanNumThousands += romanSymbols[6];
}
}
}
let finalRomanNumeral = romanNumThousands + romanNumHundreds + romanNumTens + romanNumOnes;
return finalRomanNumeral
}

``````

There is quite some room for improvement, but it’s totally fine - everything comes with experience. My suggestion, before going purely imperative with all `if ... else` statements, try to remember if there are any pre-built native JS solutions or methods that would solve the problem. For example, consider this:

``````const [one = 0, ten = 0, hrd = 0, tsd = 0] = [...num.toString()]
.reverse()
.map((n, i) => +n * 10 ** i);
const numSplit = { one, ten, hrd, tsd };
``````

…a lot of useful functionality is already pre-built making JS (one of) the most powerful functional programming language

1 Like

My second advice - look for patterns. In problem solving (and math in general) “pattern” is a synonym of “solution”. For example, convert this to Roman Numbers:

``````_  _  1  2  3    // I
4  5  6  7  8    // V
9 10             // X
...
_   _ 10 20 30    // X
40  50 60 70 80    // L
90 100             // C
``````

Look similar, aren’t they?

1 Like

Thank you, I will look into my code again and refactor it!

Спасибо!

I have refactored my code and was able to reduce lines of code to twice less the original size.

``````function convertToRoman(num) {
// only works up to 9999
const [ones = 0, tens = 0, hundreds = 0, thousands = 0] = [...num.toString()]
.reverse()
.map((n, i) => +n * 1 ** i);
const numSplit = { ones, tens, hundreds, thousands };

const romanSymbols = [['I', 'V', 'X'], ['X', 'L', 'C'], ['C', 'D', 'M'], ['M']];
let romanNumsSeparated = ["", "", "", ""];

function indNumConverter (groupNum, currVal) {
if (currVal < 4) {
for (let i=0; i < currVal; i++) {
romanNumsSeparated[groupNum] += romanSymbols[groupNum][0];
}
} else if (currVal == 4) {
romanNumsSeparated[groupNum] = romanSymbols[groupNum][0] + romanSymbols[groupNum][1];
} else if (currVal > 4 && currVal < 9) {
romanNumsSeparated[groupNum] = romanSymbols[groupNum][1];
for (let i=5; i < currVal; i++) {
romanNumsSeparated[groupNum] += romanSymbols[groupNum][0];
}
} else {
romanNumsSeparated[groupNum] = romanSymbols[groupNum][0] + romanSymbols[groupNum][2];
}
}

for (let prop in numSplit) {
if (prop == 'ones') {
indNumConverter(0, numSplit[prop]);

} else if (prop == 'tens') {
indNumConverter(1, numSplit[prop]);

} else if (prop == 'hundreds') {
indNumConverter(2, numSplit[prop]);

} else if (prop == 'thousands') {
for (let i=0; i < numSplit[prop]; i++) {
romanNumsSeparated[3] += romanSymbols[3][0];
}
}
}
return romanNumsSeparated
.reverse()
.join('');
}

convertToRoman(36);
``````

Could you please explain what does the “map” method is doing in this case specifically? I don’t quite understand what “+n * 10 ** i” is doing in this code.

`+n` converts `n` value to a number, it’s basically the same as doing `Number(n)`
`10 ** i` is the same as `Math.pow(10, i)`, so if you have number `324`, you’ll first make it a string, then array, then reverse it, so you’ll have `['4', '2', '3']`. Then if you multiply each digit to `10 ** index`, you’ll have:
``````[