Roman Numeral Converter

1

Hi fellow campers,

I’m working the Roman Numeral Converter Challenge and it looks like I am getting the correct output with my code, but for some reason the tests are not passing. Here is my code:

var answerArray = [];

function convertToRoman(num) {
var numbers = [1, 4, 5, 9, 10, 40, 50, 90, 100, 400, 500, 900, 1000];
var romanNumerals = [“I”, “IV”, “V”, “IX”, “X”, “XL”, “L”, “XC”, “C”, “CD”, “D”, “CM”, “M”];
var tempArray = [];
var lastNum;

numbers.forEach(function(value){
if(value <= num){
tempArray.push(value);
}
});

lastNum = tempArray[tempArray.length - 1];
num = num - lastNum;
answerArray.push(romanNumerals[tempArray.length - 1]);
return num > 0 ? convertToRoman(num) : answerArray.join('');

}

convertToRoman(36);

Can anyone see why my tests are not passing???

2

After a little more searching around, I found out that the algorithm challenges will not pass if you use a global variable. I changed the above solution to remove the recursion call and used a while loop instead.

1 Like
4

What u think to reverse your numbers and romanNumerals arrays, so u dont have (edit: recursive) loop or recursion?

5

On initial thought, I don’t see how the solution would be any different if the numbers and romanNumerals arrays were reversed. I’ll have to play around with it and see how I can refactor my code. Thanks!

7

Sorry, i was thinking on ‘recursive loop’, not ordinal loop to iterate through array

8

Oh I see. Yeah, I removed the recursion from my solution and added a while loop. I tried using forEach with the while loop and I got an error, so went with the for loop instead. New solution:

function convertToRoman(num) {
var numbers = [1, 4, 5, 9, 10, 40, 50, 90, 100, 400, 500, 900, 1000];
var romanNumerals = [“I”, “IV”, “V”, “IX”, “X”, “XL”, “L”, “XC”, “C”, “CD”, “D”, “CM”, “M”];
var tempArray = [];
var answerArray = [];

while(num > 0){
tempArray = [];
for(var i = 0; i < numbers.length; i ++){
if(numbers[i] <= num){
tempArray.push(numbers[i]);
}
}
num -= tempArray[tempArray.length - 1];
answerArray.push(romanNumerals[tempArray.length - 1]);
}
return answerArray.join(’’);
}

convertToRoman(29);

9

I remove that from my answer, cause with forEach u ned to acctually iterate with every single value in aray

1 Like
10

If u go from largest value to smaller, u can add up values so u dont have to go every single time again from start (smaller one in ur case)

11

For me there is no much difference between recursion and this two loop u made

12

Yeah, I guess I wasn’t trying to completely change my code. I was just trying to get the tests to pass with the code I already had. I’ll give your approach a try though when I have some more time to play around with this challenge. Thanks!

1 Like
14

Prety Good. @LauraSilvey let me add this code to my codepen if you don’t mind. You can find it at http://codepen.io/glgeorgiou/pen/BzGmRg.

16

Prety Good. @LauraSilvey let me add this code to my codepen if you don’t mind. You can find it at http://codepen.io/glgeorgiou/pen/BzGmRg.

17

@P1xt i did not mean that. I just want to practice on your code. So, i remove it. I’m sorry.

19

@P1xt may i ask you that. Why did you use this sequence (1,4,5,9) by adding a zero in each iteration. It seems something like fibonaci style :-). I just try to understand your logic.
In addition. The num value inside the while loop is not declared and so in first time has a null value. Why that?
Thank you.

21

So, if i wich to make something similar by converting to the Greek arithmetic system i have to find a different logic. Thank you @P1xt :slight_smile: