Roman Numeral Converter - solution not accepted

JavaScript
1

Tell us what’s happening:
Hello,

My first post, asking for help, so please be gentle :wink:
The code I’ve written returns the desired result, I’m not figuring out why it does not accept my solution.

Can you please help me understand?

Tks in advance,

LAsan

Your code so far
‘’’
var roman = “”;
var m = “M”; //1000
var d = “D”; //500
var c = “C”; //100
var l = “L”; //50
var x = “X”; //10
var v = “V”; //5
var i = “I”; //1
var cm = “CM”; //900
var cd = “CD”; //400
var xc = “XC”; //90
var xl = “XL”; //40
var ix = “IX”; //=9
var iv = “IV”; //=4

function convertToRoman(num) {
// num >1000
var mTimes = Math.floor(num/1000);
num = num - (mTimes*1000);
roman += m.repeat(mTimes);

// if (num > 900) {
  var cmTimes = Math.floor(num/900);
  num = num - (cmTimes*900);
  roman += cm.repeat(cmTimes);


// if (num > 500) {
  var dTimes = Math.floor(num/500);
  num = num - (dTimes*500);
  roman += d.repeat(dTimes);


// if (num > 400) {
  var cdTimes = Math.floor(num/400);
  num = num - (cdTimes*400);
  roman += cd.repeat(cdTimes);


// if (num > 100) {
  var cTimes = Math.floor(num/100);
  num = num - (cTimes*100);
  roman += c.repeat(cTimes);


// if (num > 90) {
  var xcTimes = Math.floor(num/90);
  num = num - (xcTimes*90);
  roman += xc.repeat(xcTimes);


// if (num > 50) {
  var lTimes = Math.floor(num/50);
  num = num - (lTimes*50);
  roman += l.repeat(lTimes);


// if (num > 40) {
  var xlTimes = Math.floor(num/40);
  num = num - (xlTimes*40);
  roman += xl.repeat(xlTimes);


// if (num > 10) {
  var xTimes = Math.floor(num/10);
  num = num - (xTimes*10);
  roman += x.repeat(xTimes);


// if (num > 9) {
  var ixTimes = Math.floor(num/9);
  num = num - (ixTimes*9);
  roman += ix.repeat(ixTimes);


// if (num > 5) {
  var vTimes = Math.floor(num/5);
  num = num - (vTimes*5);
  roman += v.repeat(vTimes);


// if (num > 4) {
  var ivTimes = Math.floor(num/4);
  num = num - (ivTimes*4);
  roman += iv.repeat(ivTimes);


// if (num > 1) {
  var iTimes = Math.floor(num/1);
  num = num - (iTimes*1);
  roman += i.repeat(iTimes);


  return roman;

}

convertToRoman(1004);

**Your browser information:**

Your Browser User Agent is: ```Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36```.

**Link to the challenge:**
https://www.freecodecamp.org/challenges/roman-numeral-converter
2

It looks like you simply forgot to define the variable “roman”. Try and add roman ="" at the top of your code, it should do the trick :slight_smile:

3

That’s weird because in my code I’ve declared the roman variable, but its not showing in the forum post…

Thanks, for the suggestion, but still not working… When I CTRL+ENTER the exercise console shows the desired result however doesn’t validate:

image
image1041×545 34.4 KB

6

Great, thanks for the help.
I am still struggling to understand the scope of the variables (Global, var, let, etc…)

7

Do you have some examples of what you do not understand in terms of variable scope? I will do my best to explain why certain code behaves a certain way because of scoping.

On a side note, your solution has a lot of duplicate code. One way to make your code DRY (Do not Repeat Yourself) keeping your original logic in place, is to create an array of objects which replaces the need for the global variables you created at the top of your solution. I arranged the objects in the array to correspond to the order you had all those almost duplicate code sections in. Next, I used the reduce function instead of a for loop.

function convertToRoman(num) {
  var romanNumerals = [
    {sym: 'M', val: 1000}, {sym: 'CM', val: 900}, {sym: 'D', val: 500},
    {sym: 'CD', val: 400}, {sym: 'C', val: 100}, {sym: 'XC', val: 90},
    {sym: 'L', val: 50}, {sym: 'XL', val: 40}, {sym: 'X', val: 10},
    {sym: 'IX', val: 9}, {sym: 'V', val: 5}, {sym: 'IV', val: 4}, 
    {sym: 'I', val: 1},
  ];
  return romanNumerals.reduce(function(roman, numeral) {
    var numeralTimes = Math.floor(num / numeral.val);
    num -= numeralTimes * numeral.val;
    return roman += numeral.sym.repeat(numeralTimes);
  }, '');
}

Another similar approach which uses an array of arrays instead of an array of objects looks like:

function convertToRoman(num) {  
  var romanNumerals = [
    ['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]
  ];

  return romanNumerals.reduce(function(roman, numeral) {
    var [sym, val] = numeral;
	var numeralTimes = Math.floor(num / val);
   	  num -= numeralTimes * val;
	  return roman += sym.repeat(numeralTimes);
	}, '');
}
8

Wow! Very compact solutions. Took me a while to understand because it involves some concepts I’m not so familiar with (had to revisit “reduce()”).
I tend to avoid working with arrays/objects because I usually find it difficult to access them (E.g complex JSON files), although I’m aware I should improve this.

Regarding the variable scopes, right now I don’t have a specific example, but as soon as I do I will let you know.

Again, thanks a lot for the enlightenment!

9

I have shorter, because I realized, it is no a decimal system, it is 5 digit. and first there is the same repeat III the group two is prefix. only you need if statement for IV
one is I,X,C, M 1,10,100,1000

two is V,L,D, 5,50,500
so the one code will work with every numbers