JavaScript Algorithms and Data Structures Projects - Roman Numeral Converter

Tell us what’s happening:

I just wrote the most disgusting code I have ever written on any platform or media ever. And the kicker, It works exactly as intended and PASSES all test cases yet fails them. Might just be a bug but I’m too proud to remodel the abomination I have created. You can run it yourself to see it passes all user test cases if you want. Any ideas to fix test cases?

Your code so far

let ans = "";

function convertToRoman(num) {
if (num === 0) {
  console.log(ans)
  return ans;
}
if (num >= 1000) {
  ans += 'M';
  convertToRoman(num - 1000);
} else if (num >= 900 && num < 1000) {
  ans += 'CM';
  convertToRoman(num - 900)
} else if (num >= 500 && num < 900) {
  ans += 'D';
  convertToRoman(num - 500)
} else if (num >= 400 && num < 500) {
  ans += 'CD';
  convertToRoman(num - 400)
} else if (num >= 100 && num < 400) {
  ans += 'C';
  convertToRoman(num - 100)
} else if (num >= 90 && num < 100) {
  ans += 'XC'
  convertToRoman(num - 90)
} else if (num >= 50 && num < 90) {
  ans += 'L';
  convertToRoman(num - 50)
} else if (num >= 40 && num < 50) {
  ans += 'XL';
  convertToRoman(num - 40)
} else if (num >= 10 && num < 40) {
  ans += 'X';
  convertToRoman(num - 10)
} else if (num === 9) {
  ans += 'IX';
  convertToRoman(num - 9)
} else if (num >= 5 && num < 9) {
  ans += 'V';
  convertToRoman(num - 5)
} else if (num === 4) {
  ans += 'IV';
  convertToRoman(num - 4)
} else if (num >= 1 && num < 4) {
  ans += 'I';
  convertToRoman(num - 1)
}
}

convertToRoman();


Your browser information:

User Agent is: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Safari/605.1.15

Challenge Information:

JavaScript Algorithms and Data Structures Projects - Roman Numeral Converter

1 Like

Long story short: function is relying on the variable defined outside of it, instead it should return the string with converted number. Changing that will not require remodeling the abomination, more like tuning out couple details, so don’t worry :slight_smile: .

Take also a look at your else if, try to answer the question - taking as example the first condition - at which point num, when that condition is checked, can be higher or equal to 900, but at the same time not lower than 1000 ?

2 Likes

When I put the var ans inside of the function, it looks like it keeps calling an empty string forever updating the ans to an empty string. Also I do now see the if else redundancy, its a bit late here lol.

1 Like

That’s the thing, once the variable is removed from outside, function can stop relying on that one external point where answer is stored. Actually it can stop that at all.

You have here essentially recursive function. It can return the part that it did (at most one roman numeral at a time) with the result of further recursive call.

4 Likes

I have refactored my code so that the var is contained within the function. But I am still not understanding why the code does not pass any test cases. It works perfectly fine and returns a string. I feel like this has to be a bug or something.

function convertToRoman(num) {
  let ans = "";
  const roman = (num) => {
    if (num === 0) {
      console.log(ans)
      return ans;
    }
    if (num >= 1000) {
      ans += 'M';
      roman(num - 1000);
    } else if (num >= 900) {
      ans += 'CM';
      roman(num - 900)
    } else if (num >= 500) {
      ans += 'D';
      roman(num - 500)
    } else if (num >= 400) {
      ans += 'CD';
      roman(num - 400)
    } else if (num >= 100) {
      ans += 'C';
      roman(num - 100)
    } else if (num >= 90) {
      ans += 'XC'
      roman(num - 90)
    } else if (num >= 50) {
      ans += 'L';
      roman(num - 50)
    } else if (num >= 40) {
      ans += 'XL';
      roman(num - 40)
    } else if (num >= 10) {
      ans += 'X';
      roman(num - 10)
    } else if (num === 9) {
      ans += 'IX';
      roman(num - 9)
    } else if (num >= 5) {
      ans += 'V';
      roman(num - 5)
    } else if (num === 4) {
      ans += 'IV';
      roman(num - 4)
    } else if (num >= 1) {
      ans += 'I';
      roman(num - 1)
    }
  }
  return roman(num);
}

convertToRoman();

This was by far the the dumbest bug (or correctly created problem but still dumb) I have ever came across on this website. The above code is a near perfect solution. return roman(num) calls the function and THE FUNCTION ITSELF RETURNS A ROMAN STRING FROM THE BASE CASE! at the very bottom of both recursive solutions it returns ans. for the folks out there looking for how I solved it, I literally just needed to call the function and put return ans; even though the function only returns ans…

In your code above, printing to the console log is not the same as returning a value from a function. So although it may look like your function is returning a roman numeral string, it really isn’t. You can verify this by adding the following after the function definition:

console.log(convertToRoman(36));

Yes, you will see the roman numeral string stored in ans printed by the console.log in the first if block because you are basically using the ans variable as a global variable, so this is not really a true recursive function. Each call you make to roman() is an independent function call, and the only time roman() actually returns a value is when the argument passed to it is 0. That means that all those other calls to roman() don’t explicitly return a value and thus your original call to roman() returns undefined (which you will verify if you console log the output as I suggested above).

This is why you need to explicitly return ans, because it is the global variable that the roman numeral string is being built in. I suppose you could make this a real recursive function if you want, but remember, your recursive function needs to return a value every time it is called, you can’t just keep calling the function over and over again and only return a value on the base case. And you don’t need to define a second inner function to do this.

Bottom line, this was a bug with your code, not the website.

1 Like

Thank you for the insight! very useful!

Well, that’s just brilliant! Congratulations!

1 Like

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