Jump to content

JavaScript - Roman Numeral Converter

Go to solution Solved by Vicarian,

It doesn't throw an error, but doesn't give correct output if the input is a number beginning with 0.

const ints = [ 015 ];
//Expected output: XV
//Actual output: XIII

It appears ECMAScript treats this representation as an octal (base-8) number if all digits in the number are less than 8.  Are there any rules governing what input you accept?  Must the input always be a base-10 integer?  It's obviously an edge case with a small chance of happening, but if someone accidentally strikes a zero (say with number pad entry) and gets this conversion, it would be confusing.  You can trim leading zeros in a couple ways, first, you could convert the input to a string, then invoke parseInt, specifying decimal as the number base to use, then convert the output of that back to a string:
 

let strNum = parseInt(number.toString(), 10).toString();

Or, you could Regex away all zeroes at the beginning of the string:

let strNum = String(number);

strNum = strNum.replace(/^0+/, '');

Should anything happen if the input isn't a Number type?  Again, your function doesn't throw any errors, instead giving undefined output for this case:

const ints = [ 'XIII' ];

Attempting to sanitize weird user input is always something we need to do as programmers: "Garbage in, garbage out."  Depending on requirements, code working, but not alerting the user to errant inputs might not be the desired outcome.  For a beginner, the code is great, but a minor suggestion: personally, I'd prefer spaces instead of tabs for indentation.  Tab sizes vary widely, spaces generally don't.  And if you aren't consistent between tabbing or using spaces, the file might look fine in your editor, but if I'd copy it into mine, it would look awful.  You can make sure your editor of choice inserts spaces instead of tabs using its text editor settings, wherever those live.

 

Also, I looked up the exponentiation operator on the MDN.  While this overload enjoys wide support, it's not supported by older versions of browsers.  If your "customer" would be using old hardware/software, the code wouldn't work (at all, or as expected, depending on platform-specific error handling).  Platform support for what you write is also something to be cognizant of if you want to enter the industry as a professional.

const ints = [];
for (i = 0; i < 10; i++) {
	ints.push(Math.floor(Math.random() * 1000 + 1));
}
const glyphs = {1000: "M", 500: "D", 100: "C", 50: "L", 10: "X", 5: "V", 1: "I"};
const toRoman = number => {
	let strNum = String(number);
	let romNum = "";
	for (i = 0; i < strNum.length; i++) {
		let currIndex = strNum.length - i - 1;
		let digit = Number(strNum[i]);
		switch (digit) {
			case 9:
				romNum += glyphs[10 ** currIndex] + glyphs[10 ** (currIndex + 1)];
				break;
			case 5:
				romNum += glyphs[5 * (10 ** currIndex)];
				break;
			case 4:
				romNum += glyphs[10 ** currIndex] + glyphs[5 * (10 ** currIndex)];
				break;
			default:
				if (digit > 5) {
					romNum += glyphs[5 * (10 ** currIndex)] + glyphs[10 ** currIndex].repeat(digit - 5);
				} else {
					romNum += glyphs[10 ** currIndex].repeat(digit);
				}
		}
	}
	return romNum;
}
ints.forEach((elem) => console.log(`Base 10: ${elem}\nRoman Numeral: ${toRoman(elem)}\n`));

Coding greenhorn. Trying to learn Js. Does this look alright?

Link to comment
https://linustechtips.com/topic/1598985-javascript-roman-numeral-converter/
Share on other sites

Link to post
Share on other sites

It doesn't throw an error, but doesn't give correct output if the input is a number beginning with 0.

const ints = [ 015 ];
//Expected output: XV
//Actual output: XIII

It appears ECMAScript treats this representation as an octal (base-8) number if all digits in the number are less than 8.  Are there any rules governing what input you accept?  Must the input always be a base-10 integer?  It's obviously an edge case with a small chance of happening, but if someone accidentally strikes a zero (say with number pad entry) and gets this conversion, it would be confusing.  You can trim leading zeros in a couple ways, first, you could convert the input to a string, then invoke parseInt, specifying decimal as the number base to use, then convert the output of that back to a string:
 

let strNum = parseInt(number.toString(), 10).toString();

Or, you could Regex away all zeroes at the beginning of the string:

let strNum = String(number);

strNum = strNum.replace(/^0+/, '');

Should anything happen if the input isn't a Number type?  Again, your function doesn't throw any errors, instead giving undefined output for this case:

const ints = [ 'XIII' ];

Attempting to sanitize weird user input is always something we need to do as programmers: "Garbage in, garbage out."  Depending on requirements, code working, but not alerting the user to errant inputs might not be the desired outcome.  For a beginner, the code is great, but a minor suggestion: personally, I'd prefer spaces instead of tabs for indentation.  Tab sizes vary widely, spaces generally don't.  And if you aren't consistent between tabbing or using spaces, the file might look fine in your editor, but if I'd copy it into mine, it would look awful.  You can make sure your editor of choice inserts spaces instead of tabs using its text editor settings, wherever those live.

 

Also, I looked up the exponentiation operator on the MDN.  While this overload enjoys wide support, it's not supported by older versions of browsers.  If your "customer" would be using old hardware/software, the code wouldn't work (at all, or as expected, depending on platform-specific error handling).  Platform support for what you write is also something to be cognizant of if you want to enter the industry as a professional.

Link to post
Share on other sites

14 minutes ago, Vicarian said:

It doesn't throw an error, but doesn't give correct output if the input is a number beginning with 0.

const ints = [ 015 ];
//Expected output: XV
//Actual output: XIII

It appears ECMAScript treats this representation as an octal (base-8) number if all digits in the number are less than 8.  Are there any rules governing what input you accept?  Must the input always be a base-10 integer?  It's obviously an edge case with a small chance of happening, but if someone accidentally strikes a zero (say with number pad entry) and gets this conversion, it would be confusing.  You can trim leading zeros in a couple ways, first, you could convert the input to a string, then invoke parseInt, specifying decimal as the number base to use, then convert the output of that back to a string:
 

let strNum = parseInt(number.toString(), 10).toString();

Or, you could Regex away all zeroes at the beginning of the string:

let strNum = String(number);

strNum = strNum.replace(/^0+/, '');

Should anything happen if the input isn't a Number type?  Again, your function doesn't throw any errors, instead giving undefined output for this case:

const ints = [ 'XIII' ];

Attempting to sanitize weird user input is always something we need to do as programmers: "Garbage in, garbage out."  Depending on requirements, code working, but not alerting the user to errant inputs might not be the desired outcome.  For a beginner, the code is great, but a minor suggestion: personally, I'd prefer spaces instead of tabs for indentation.  Tab sizes vary widely, spaces generally don't.  And if you aren't consistent between tabbing or using spaces, the file might look fine in your editor, but if I'd copy it into mine, it would look awful.  You can make sure your editor of choice inserts spaces instead of tabs using its text editor settings, wherever those live.

For the user input, couldn't I just check the truthiness of Number(userInput) and then throw an alert or something if it's not a positive integer between 1 and 3999?

To be quite honest, validating input is something I wasn't thinking too hard about because I wanted to make sure the core logic i.e. the actual conversion was working first. So like, solve this problem first and then think about how to build either around it or improve it a little.

Link to post
Share on other sites

6 minutes ago, okkee said:

For the user input, couldn't I just check the truthiness of Number(userInput) and then throw an alert or something if it's not a positive integer between 1 and 3999?

To be quite honest, validating input is something I wasn't thinking too hard about because I wanted to make sure the core logic i.e. the actual conversion was working first. So like, solve this problem first and then think about how to build either around it or improve it a little.

Exactly, I couldn't find anything wrong with your logic, so started testing edge cases.  In terms of truthiness, Number(015) returns 13 (the decimal of the octal input), so the truthiness check wouldn't necessarily suffice.  You'd have to impose more rules than just true coming back from
 

Number.isNaN(userInput)

Like I said, great start, keep at it.

Link to post
Share on other sites

6 minutes ago, Vicarian said:

Exactly, I couldn't find anything wrong with your logic, so started testing edge cases.  In terms of truthiness, Number(015) returns 13 (the decimal of the octal input), so the truthiness check wouldn't necessarily suffice.  You'd have to impose more rules than just true coming back from
 

Number.isNaN(userInput)

Like I said, great start, keep at it.

Alright. Thank you so much. I'll have to think more about input validation.

Link to post
Share on other sites

18 hours ago, okkee said:

Alright. Thank you so much. I'll have to think more about input validation.

In this case you might be able to get away with a simple regex that only accepts 0 or 1-9 followed by 0-9.

 

0|[1-9][0-9]*

 

This should do the trick? Haven't tested it yet though

Remember to either quote or @mention others, so they are notified of your reply

Link to post
Share on other sites

You could just use parseInt(value,  base)  ex  parseInt("0123", 10) = 123

 

As for the code, you could really simplify it by adding the special cases as glyphs (ex 900, 400, 90, 40, 9, 4)

 

Here's example code in php, could be easily translated to javascript :

 

<?php

$nr = 660;

$glyphs = array(1000 => "M", 900 => "CM", 500 => "D", 400=>"CD", 
				100=> "C", 90 => "XC", 50=> "L", 40 =>"XL", 
				10=> "X", 9=> "IX", 5=> "V", 4=> "IV", 1=>"I");
$roman  = '';				
while ($nr>0) {
	$continue=true; 
    // using a boolean to avoid using the break command 
    // the number of elements in the glyphs is small enough i don't care 
    // a few cpu cycles are wasted by looping through all elements
    // it makes for easier code to read and debug
	foreach ($glyphs as $value => $text) {
		if (($continue==true) && ($nr >= $value)) {
			$roman .= $text; // append glyph to text
			$continue=false; // ignore the remaining glyphs
			$nr = $nr - $value; // update number 
		}
	}
}
echo $roman;

?>

 

Link to post
Share on other sites

40 minutes ago, mariushm said:

As for the code, you could really simplify it by adding the special cases as glyphs (ex 900, 400, 90, 40, 9, 4)

Thanks for the feedback. It's been a while but this is the basically finished version:

JS:

const userNumBox = document.getElementById("number");
const resultBox = document.getElementById("output");
const submitBtn = document.getElementById("convert-btn");
const rulesText = document.getElementById("rules")
const romanTable = document.getElementById("rules-table")


const glyphs = [
	[1000, "M"], [900, "CM"], [500, "D"], [400, "CD"], [100, "C"],
	[90, "XC"], [50, "L"], [40, "XL"], [10, "X"], 
	[9, "IX"], [5, "V"], [4, "IV"], [1, "I"]
	];

const toRoman = num => {
	//Declaration of return string
	let convNum = num;
	//Check input
	if (convNum === "") {
		return "Please enter a valid number";
	}
	if (convNum > 3999) {
		return "Please enter a number less than or equal to 3999";
	}
	if (convNum < 0) {
		return "Please enter a number greater than or equal to 1";
	}

	let retStr = "";
	//for...of loop using deconstruction for doubles
	for (const [key, value] of glyphs) {
		while (convNum >= key) {
			//while loop (for later - maybe just change to if?)
			//adds symbol n times to return string where n equals the number of times it is divisible by the int
			//changes num to remainder i.e ends current iteration
			retStr += value.repeat(Math.floor(convNum/key));
			convNum %= key;
		}
	}
	return retStr;
};

submitBtn.addEventListener("click", (e) => {
	e.preventDefault()
  resultBox.innerText = toRoman(userNumBox.value);
  rulesText.style.display = "none";
  romanTable.style.display = "none";
  resultBox.style.display = "block";
})

HTML:

<!DOCTYPE html>
<html lang="en">
  <head>
    <link rel="stylesheet" href="styles.css" />
    <meta charset="utf-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
  </head>
  <body>
    <section id="section-1">
      <h1 class="main-title">Roman Numeral Convertor</h1>
      <form id="submit-form">
        <label for="number">
        Enter Number:
          <input type="number" id="number" placeholder="Enter number here" required />
        </label>
      <button type="submit" id="convert-btn">
          Submit
      </button>
      </form>
    </section>
    <section id="section-2">
      <div id="output"></div>
      <p id="rules">
        Roman numerals are based on seven symbols and can be written using various combinations to represent Arabic numerals.For Example:
      </p>
      <table id="rules-table">
        <caption>Roman Numeral Table</caption>
        <tr>
          <th>Roman Numerals</th>
          <th>
            Arabic Numerals
          </th>
        </tr>
        <tr>
          <td>M</td>
          <td>1000</td>
        </tr>
        <tr>
          <td>CM</td>
          <td>900</td>
        </tr>
        <tr>
          <td>D</td>
          <td>500</td>
        </tr>
        <tr>
          <td>CD</td>
          <td>400</td>
        </tr>
        <tr>
          <td>C</td>
          <td>100</td>
        </tr>
        <tr>
          <td>XC</td>
          <td>90</td>
        </tr>
        <tr>
          <td>L</td>
          <td>50</td>
        </tr>
        <tr>
          <td>XL</td>
          <td>40</td>
        </tr>
        <tr>
          <td>X</td>
          <td>10</td>
        </tr>
        <tr>
          <td>IX</td>
          <td>9</td>
        </tr>
        <tr>
          <td>V</td>
          <td>5</td>
        </tr>
        <tr>
          <td>IV</td>
          <td>4</td>
        </tr>
        <tr>
          <td>I</td>
          <td>1</td>
        </tr>
      </table>
    </section>
    <script src="script.js"></script>
  </body>
</html>

I've left out the styling but yeah, this is basically how it's looking now.

 

EDIT:

- if (convNum < 0) should be, if (convNum <= 0)

Edited by okkee
Fixing one of the lines
Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×