Roman Numeral Converter not passng the test

Programming
1

Greetings dears,
Thank you for the amazing toturials and challenges,

I’m doing the below challange Roman Numeral Converter , I tested it and it prints the correct number, however it doesn’t pass the test,

I have no clue what am I doing wrong in this challange, do you have some pointers?
Thank you in advance.

   **My code so far**


const keys = {
 M:1000,
 CM:900,
 D:500,
 CD:400,
 C:100,
 XC:90,
 L:50,
 XL:40,
 X:10,
 IX:9,
 V:5,
 IV:4,
 I:1
 }

let tempRoman = ""

function convertToRoman(num) {
 if (num <= 0) {
   return tempRoman
 }

for(const [i, val] of Object.entries(keys) ){
 if(num >= val){
   tempRoman += i
   return convertToRoman(num - val)
 }
}
return tempRoman;

}

console.log(convertToRoman(3999))
convertToRoman(3999)

   **Browser information:**

User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/94.0.4606.81 Safari/537.36

Challenge: Roman Numeral Converter

Link to the challenge:

2

tempRoman is a global variable - what happens to it on the second and subsequent tests? Does it clear between tests, or continue to accumulate?

3

Thank you dear for trying to help,
It’s actually an accumulative variable, it doesn’t clear on any of the iterations ,

I tested it my self :cry:

4

And that’s the problem. You’re testing with one test, and that may work - but the test suite runs a few tests in series, and it must pass them all.

You have a few ways to do this, but they all come down to…don’t use a global variable.

5

This is helpfull,
but I don’t know why not to use a global variable !!
I’ve changed my code a little bit, and now it passed :v:

still don’t under stand why not to use the global vairable ?

solution below :

const keys = {
  "M":1000,
  "CM":900,
  "D":500,
  "CD":400,
  "C":100,
  "XC":90,
  "L":50,
  "XL":40,
  "X":10,
  "IX":9,
  "V":5,
  "IV":4,
  "I":1
  }

// Removed the gloval variable, and provide it as function parameter with a default to empty string

function convertToRoman(num,tempRoman="") {

  if (num <= 0) {
    return tempRoman 
  }

for(const [i, val] of Object.entries(keys) ){
  if(num >= val){
  
    tempRoman = tempRoman + (i)
// recursively reduce the number, and provide the string at current state
    return convertToRoman(num - val, tempRoman)
  }
}
}

console.log(convertToRoman(29))
6

This is not a great challenge to use recursion on, in my opinion. It makes for convoluted logic. You can just replace the recursive call with a while loop.

WRT global variables…

Example:

var myGlobal = [1];
function returnGlobal(arg) {
  myGlobal.push(arg);
  return myGlobal;
} // unreliable - array gets longer each time the function is run

function returnLocal(arg) {
  var myLocal = [1];
  myLocal.push(arg);
  return myLocal;
} // reliable - always returns an array of length 2
7

If a local variable is used rather than a global, each time the function is run it should be generating a new value. By using a global that never gets cleared between tests, the value is simply being combined with what is already in that global.

8

Ahaaaaa, I got it now
So basically, the test is getting the value from the previous state of the global variable
The test, does not re run and clear all values from the beginning,
I got it now
Soooo much appreciated boss :tada::star2:

9

This is understandable dear
I will try to make another algorithm for it using a while loop
Thank you for your contribution
:star2:

10

Greetings dears,

I have done another logic using while loop, as per your recommendation


function convertToRoman(num) {
const keys = {
  "M":1000,
  "CM":900,
  "D":500,
  "CD":400,
  "C":100,
  "XC":90,
  "L":50,
  "XL":40,
  "X":10,
  "IX":9,
  "V":5,
  "IV":4,
  "I":1
  }

  let tempRoman = ""
  let tempNum = num

  while(tempNum >= 0){
    if(tempNum <= 0) return tempRoman

   
    for (let [roman,val] of Object.entries(keys)){
        if(tempNum >= val) {
          tempRoman += roman
          tempNum-=val
          break;
        }
    }
  
  }
}

console.log(convertToRoman(29))