Roman Numeral Converter

I believe I have solved this challenge, however the below solution is failing every test. Even when i console.log and get the correct answer. Please can someone help

let romanNumerals =
function convertToRoman(num) {

if (num === 0){
    return romanNumerals.join("")
}    
else if (num >= 1000){
    romanNumerals.push("M")
    num -= 1000
     return convertToRoman(num)
}
else if (num >= 900){
    romanNumerals.push("CM")
    num -= 900
    return convertToRoman(num)
}
else if (num >= 500){
    romanNumerals.push("D")
    num -= 500
    return convertToRoman(num)
}
else if (num >= 400){
    romanNumerals.push("CD")
    num -= 400
    return convertToRoman(num)
}
else if (num >= 100){
    romanNumerals.push("C")
    num -= 100
    return convertToRoman(num)
}
else if (num >= 90){
    romanNumerals.push("XC")
    num -= 90
    return convertToRoman(num)
}
else if (num >= 50){
    romanNumerals.push("L")
    num -= 50
    return convertToRoman(num)
}
else if (num >= 40){
    romanNumerals.push("XL")
    num -= 40
    return convertToRoman(num)
}
else if (num >= 10) { 
    romanNumerals.push("X")
    num -= 10
    return convertToRoman(num)
}
else if (num >=9){
    romanNumerals.push("IX")
    num -= 9
    return convertToRoman(num)
}
else if (num >=5) {
    romanNumerals.push("V")
    num -= 5
    return convertToRoman(num)
}
else if (num == 4){
    romanNumerals.push("IV")
    num -= 4
    return convertToRoman(num)

}
else if (num >=1){
    romanNumerals.push("I")
    num -=1
    return convertToRoman(num)
}

}

console.log(convertToRoman(900))

Hi @JXG052 Welcome to the forum!

If you have a question about a specific challenge as it relates to your written code for that challenge, just click the Ask for Help button located on the challenge. It will create a new topic with all code you have written and include a link to the challenge also. You will still be able to ask any questions in the post before submitting it to the forum.

Thank you.

Tell us what’s happening:
I believe the below code works ( i appreciate it could be cleaner) however it fails all tests when I submit. Please can someone advise what I am missing?

Thanks in advance!

Your code so far


let romanNumerals = []


function convertToRoman(num){
    
    if (num === 0){
        let romanNumeralsStr = romanNumerals.join("")
        return romanNumeralsStr
    }    
    else if (num >= 1000){
        romanNumerals.push("M")
        num -= 1000
         return convertToRoman(num)
    }
    else if (num >= 900){
        romanNumerals.push("CM")
        num -= 900
        return convertToRoman(num)
    }
    else if (num >= 500){
        romanNumerals.push("D")
        num -= 500
        return convertToRoman(num)
    }
    else if (num >= 400){
        romanNumerals.push("CD")
        num -= 400
        return convertToRoman(num)
    }
    else if (num >= 100){
        romanNumerals.push("C")
        num -= 100
        return convertToRoman(num)
    }
    else if (num >= 90){
        romanNumerals.push("XC")
        num -= 90
        return convertToRoman(num)
    }
    else if (num >= 50){
        romanNumerals.push("L")
        num -= 50
        return convertToRoman(num)
    }
    else if (num >= 40){
        romanNumerals.push("XL")
        num -= 40
        return convertToRoman(num)
    }
    else if (num >= 10) { 
        romanNumerals.push("X")
        num -= 10
        return convertToRoman(num)
    }
    else if (num >=9){
        romanNumerals.push("IX")
        num -= 9
        return convertToRoman(num)
    }
    else if (num >=5) {
        romanNumerals.push("V")
        num -= 5
        return convertToRoman(num)
    }
    else if (num == 4){
        romanNumerals.push("IV")
        num -= 4
        return convertToRoman(num)

    }
    else if (num >=1){
        romanNumerals.push("I")
        num -=1
        return convertToRoman(num)
    }
}

console.log(convertToRoman(44))


Your browser information:

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

Challenge: JavaScript Algorithms and Data Structures Projects - Roman Numeral Converter

Link to the challenge:

Your code contains global variables that are changed each time the function is run. This means that after each function call completes, subsequent function calls start with the previous value. To fix this, make sure your function doesn’t change any global variables, and declare/assign variables within the function if they need to be changed.

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

Don’t use a global variable here. The tests run sequentially, so romanNumerals at the start of a new test will still be the same value at the end of the previous test. This is an example of why you should try to avoid global variables when all possible.

multiple people have already identified the issue of using global variables, but I just wanted to add that if you really want to keep track of the array and use recursion, then I suggest you make a ‘helper’ method that convertToRoman(num) can call so as to keep track of the array. Possibly something like convertToRomanHelper(num, romanNumerals).

Also, I’d recommend using switch as it’s cleaner than loads of if/else statements.

Thanks all, I appreciate the help - although I must admit I don’t really understand what the problem with using a global variable is? If I declare the array locally within the function won’t it just reset with every recursion?

You could also use true recursion and use the actual return value. Recursion that cheats with some global variable or internal helper function is really decreasing clarity is most cases.

Really though, I think a loop is simpler here.

What happens if you want to call the function twice? With the first function call, you will have filled the global variable with ‘junk’ that’s not relevant to the second function call.

Yes. So you should return data you need from each function call.

yes it will reset.
If you want to keep the exact code with the recursion intact, you must use a helper function that you can call from the main function and pass the empty array to it so it can recurse using that array and fill it up as it goes.

aah ok, yh that makes sense. Thanks everyone :slight_smile:

I really recommend against the inner helper function to cheat funky recursive functions.

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