Roman Numeral Converter | Javascript Project 2

Hi,

I am not sure why the code is not working. When I log out the required str, it works how it should, but the test aren’t passing.

const romanToArabic = {
  //Testing 1 to 10 numerals
  I : 1,
  IV : 4,
  V : 5,
  IX: 9,
  X: 10
}

 let str = "";
function convertToRoman(num) {
  let numeralWithLeastDifference;

  for(let item in romanToArabic){  
    if(num === romanToArabic[item]){
       str += item;
      console.log(str)
      return str
    } 
     else if (num > romanToArabic[item]){
      numeralWithLeastDifference = romanToArabic[item]
      
     }  
  }
  //console.log(numeralWithLeastDifference)
 for (let item in romanToArabic){  
   if (numeralWithLeastDifference === romanToArabic[item]){
     str+= item
     num = num -  numeralWithLeastDifference;

     convertToRoman(num)
    
   }
 } 

}

convertToRoman(8);

Link to the Problem- https://www.freecodecamp.org/learn/javascript-algorithms-and-data-structures/javascript-algorithms-and-data-structures-projects/roman-numeral-converter

Thanks,
Deeti

Hey Deeti!

if you look at the failed test cases, this is what it says.
image

they expect you to return a string but in most cases you aren’t returning anything.

image

and i believe this is because of this condition. until or unless the number isn’t directly equal to the roman numeral you arent returning anything.

image

Hi,

if (numeralWithLeastDifference === romanToArabic[item])

After this point, I am concocting the string with the item, then I am calling convertToRoman again.
I put the return function when required roman numerals are added and then in the end, when the str matches with the item, I will return it.
I don’t really understand where I am making mistake.

Thanks for gettingback @staranbeer :star:

Hi
some tests
need to add more console-logging to troubleshoot tho
for now it looks like you are adding too much symbols for some numbers
and for others you’re not returning anything

pattern
undefined is the result for cases which are not present in the object

const romanToArabic = {
  //Testing 1 to 10 numerals
  I : 1,
  IV : 4,
  V : 5,
  IX: 9,
  X: 10
}

 let str = "";
function convertToRoman(num) {
  let numeralWithLeastDifference;

  for(let item in romanToArabic){  
    if(num === romanToArabic[item]){
       str += item;
      //console.log(str)
      return str
    } 
     else if (num > romanToArabic[item]){
      numeralWithLeastDifference = romanToArabic[item]
      
     }  
  }
  //console.log(numeralWithLeastDifference)
 for (let item in romanToArabic){  
   if (numeralWithLeastDifference === romanToArabic[item]){
     str+= item
     num = num -  numeralWithLeastDifference;

     convertToRoman(num)
    
   }
 } 

}

for (let i = 1; i < 10; i++) {
  console.log(i, convertToRoman(i));
}

/*
Output:

1 I
2 undefined
3 undefined
4 IIIIIIIV
5 IIIIIIIVV
6 undefined
7 undefined
8 undefined
9 IIIIIIIVVVIVIIVIIIIX
*/

No worries. I’m always happy to help!

I understand the algorithms. i was just pointing out that the “VIII” you see in the output is because of a console log. when i log the function call it returns undefined and the tests check what your function returns and not what it logs.

image

i think the problem may be on this line because you aren’t doing anything with the value it returns.

I take this back :joy: i just tried to run your code and it should work but it isnt so imma keep trying until i figure it out.

I am planning to refactor my own solution for this, so

I took fcc tests cases and re-wrote them a little to make output of the function more readable

May be helpful:
(i double checked this stuff for typos, so I guess you can just paste it below your function and run the code if you want to use it for testing)

//TESTS

const testCases = {
  
  'onedigit': [[2, 'II'], [3, 'III'], [4, 'IV'], [5, 'V'], [9, 'IX']],
  
  'twodigit': [[12, 'XII'], [16, 'XVI'], [29, 'XXIX'], [44, 'XLIV'], [45, 'XLV']
                , [68, 'LXVIII'], [83, 'LXXXIII'], [97, 'XCVII'], [99, 'XCIX']],
                
  'threedigit': [[400, 'CD'], [500, 'D'], [501, 'DI'], [649, 'DCXLIX'],
                [798, 'DCCXCVIII'], [891, 'DCCCXCI']],
  
  'fourdigit' : [[1000, 'M'], [1004, 'MIV'], [1006, 'MVI'],
                  [1023, 'MXXIII'], [2014, 'MMXIV'], [3999, 'MMMCMXCIX']]
}

for (let group in testCases) {
  console.log('tests for ', group, ' numbers');
  for (let testCase of testCases[group]) {
    console.log('CASE: ', testCase[0]);
    console.log('expected: ', testCase[1]);
    console.log('fact: ', convertToRoman(testCase[0]));
    console.log('----')
  }
  console.log('------------');
}
1 Like

Hi @admit8490 , @staranbeer

I am not sure if I am understanding where I am making a mistake. I maybe need more details. When I run the code on console, it is returning undefined. I think what I am trying to understand is why when it log the str it shows the answer, but when I return it , it displays undefined.

In the past, I have usually log my answers and they all the time match with the returned result. This never happened before, so I am finding it very unusual.

Thanks again for helping :star2:

You have some recursive call, right? I mean here.

So you are calling function, but you don’t actually assigning output to anything.
Maybe issue connected to this.

I can’t understand details of your algo right now, I will run your code in pythontutor to wrap my mind around it.

so the logic, I am using is -
first, the function see if num matches with the values in the object,
else, I will find the the Arabic numeral which less than the num and closest t o it.

Say for num 19, 10 is the closest numeral, lesser than the num
10 will be save as numeralWithLeastDifference.
Once, I find the numeralWithLeastDifference, I will subtract it from the number, add the roman with string, and call out convertToRoman(9) because now the number is 9.

Now, str = “X”

When the function runs again, the num will match the object value 9 and
str = “X” + “IX”
str = “XIX”
and this value will be returned .

Thats how I see the function to work.

P.S @staranbeer

Ok, I get the idea I think. But still. You have only one return statement, at the top of your code.

After that you also need some return statements, right? Lower in the code, for other cases? This call

you need to do something with it to make this work.

Just recalled. I used similar approach not long ago. I was dealing with converter also, for one of the Euler challenges (I was converting numbers like 3 to words like three)

I also used recursive calls in the same fashion, but I was assigning this calls to variables, used them in expressions etc

i dont think it’ll help you at all if i just give you the answer, so here’s a hint. calculate the value of 8 using your algorithm. if possible, try to follow the execution path on pen and paper.

1 Like

I think we can’t run loop on objects.

Hello, you are referring to this?

for… in loop is exactly for looping through props of object. Or maybe I misunderstood?

Sorry, actually i was wrong.

Somehow, I can’t wrap my mind, why I need the return statement here- convertToRoman(num).
Something is off with my basics.

What confuses me more that I used to belief that console.log and return works similarly when rendering solution.

Like,

function consoling(){
console.log("Hey")
}
consoling()

AND

function returning(){
return "Hey"
}
returning()

Thanks,
Deeti

Let me try that.
Thanks

Are you familiar with this tool below?

https://pythontutor.com/visualize.html#mode=edit

It runs your code line by line. It’s incredibly useful to trouble shoot some recursion-related problems.

Consider to run your code there.

It can be a good addition to pen and paper execution mentioned above.

well, that’s different things: console.log and return

In theory, yes. But when I apply them, they kind of worked similarly. At least that has been my biased experience. hehe