Arabic to Roman Project

Code Feedback
1

Hello there.

I just finished this project. It seems to work as intended, for numbers that are not too large anyway(there seem to be debates on how to write 4000, but the tests are all <4000 anyway).

I’d love some feedback on this project. What 's good, what could be better? I have mixed feelings about using the romanGroups object, i feel like i could have done all the replacements in romanStr using just a regex. What about using numbers as object keys? Is it fine? The function does its job, but i don’t remember seeing anyone use numbers for object keys. Also i’m not sure if the regex i used is the best there is.

What are your thoughts?
Any feedback is appreciated.

  **Your code so far**


function convertToRoman(num) {
let romans = {
  1000:'M',
  500:'D',
  100:'C',
  50:'L',
  10:'X',
  5:'V',
  1:'I'
}
let romanGroups = {
  'IIII': 'IV',
  'VIIII': 'IX',
  'XXXX': 'XL',
  'LXXXX': 'XC',
  'CCCC': 'CD',
  'DCCCC': 'CM'
}
 //the equivalents of roman numerals
let nums = [1000, 500, 100, 50, 10, 5, 1];
 //used to iterate over the nums array, from the most significant term(M), to the least significant(I)
let i = 0;
 //container for the numbers that make up the function argument when added
let numArr = [];
 //here happens the decomposing of the function argument into nums array terms
while(num > 0){ 
  if(num - nums[i] >= 0){
    numArr.push(nums[i]);
    num-=nums[i];
  } else {
    i++;
  }
}
 //converts the array from arabic to roman and joins it into a string
let romanStr = numArr.map(item => romans[item]).join('');
 //however, the string will represent 4 as 'IIII', 9 as 'VIIII' and so on
//glorious regex to match the 'weird' sequences
let matches = romanStr.match(/(DCCCC|LXXXX|VIIII)|(I{4}|X{4}|C{4})/g); 
if(matches){
  matches.forEach(item => {
    romanStr = romanStr.replace(item, romanGroups[item])
  });
}
return romanStr;
}

convertToRoman(3999);

  **Your browser information:**

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

Challenge: Roman Numeral Converter

Link to the challenge:

2

It’s not how I would have done it, but I think that swapping out sets of 4 is clever. If you don’t want to have a separate romans and romansGroups, then you can rearrange your logic a little bit to make numbers like VI or XI a unit in your romans array.

3

You mean to add 4: 'IV', 9: 'IX' and so on, to the romans object and remove romanGroups altogether, right? Good idea, i can see how that would work.

1 Like
4

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