HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Roman Numerals - the good, the bad, and the ugly

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
theromanuglybadnumeralsgoodand

Problem

I wrote a RomanNumeral class in C#. Please pull it to pieces:

class RomanNumeral
{
public RomanNumeral(string num)
{
AsRoman = num;
AsArabic = RomanNumeralToArabic(num);

if (AsArabic == -1)
{
throw new ArgumentException();
}
}

public RomanNumeral(int num)
{
AsRoman = ArabicNumeralToRoman(num);
AsArabic = num;

if (AsRoman == "")
{
throw new ArgumentException();
}
}

private string ArabicNumeralToRoman(int num)
{
string val = "";

while (num >= 1000)
{
val += "M";
num -= 1000;
}
if (num >= 900)
{
val += "CM";
num -= 900;
}
while (num >= 500)
{
val += "D";
num -= 500;
}
if (num >= 400)
{
val += "CD";
num -= 400;
}
while (num >= 100)
{
val += "C";
num -= 100;
}
if (num >= 90)
{
val += "XC";
num -= (0;
}
while (num >= 50)
{
val += "L";
num -= 50;
}
if (num >= 40)
{
val += "XL";
num -= 40;
}
while (num >= 10)
{
val += "X";
num -= 10;
}
if (num >= 9)
{
val += "IX";
num -= 9;
}
while (num >= 5)
{
val += "V";
num -= 5;
}
if (num >= 4)
{
val += "IV";
num -= 4;
}
while (num >= 1)
{
val += "I";
num -= 1;
}

return val;
}

private int RomanNumeralToArabic(string num)
{
int val = -1;

for (int i = 0; i

I really hate that humongous
switch in RomanNumeralToArabic`. Should that be a

Solution

Note: I don't know any C#, so I'm not able to provide any code samples. All code is pseudo-code.

The way I would implement this is using a hash map and a while loop. The hash map links arabic numerals to roman numerals:

TRANSLATIONS = { 
  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"
}


You can then loop through the hash map (from highest key to lowest key). You can check how many times the current numeral should be displayed by dividing. You can find out how to display the rest by using the modulo operation (% in most languages).

number = 64
rest   = number
result = ""

for arabic, numeral in map {
  times = rest / arabic
  rest  = rest % arabic # modulo

  for i = 0, i < times; i = i + 1 {
    result = result + numeral
  }

  if (rest == 0) {
    break
  }
}

return result


I hope this is useful :)

Code Snippets

TRANSLATIONS = { 
  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"
}
number = 64
rest   = number
result = ""

for arabic, numeral in map {
  times = rest / arabic
  rest  = rest % arabic # modulo

  for i = 0, i < times; i = i + 1 {
    result = result + numeral
  }

  if (rest == 0) {
    break
  }
}

return result

Context

StackExchange Code Review Q#80545, answer score: 6

Revisions (0)

No revisions yet.