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 < num.Length; i++)
{
switch (num[i])
{
case 'M':
val += 1000;
break;
case 'D':
val += 500;
break;
case 'C':
if (i != num.Length - 1)
{
switch (num[i + 1])
{
case 'M':
case 'D':
val -= 100;
break;
default:
val += 100;
break;
}
}
else
{
val += 100;
}
break;
case 'L':
val += 50;
break;
case 'X':
if (i != num.Length - 1)
{
switch (num[i + 1])
{
case 'M':
case 'D':
return -1;
case 'L':
case 'C':
val -= 10;
break;
default:
val += 10;
break;
}
}
else
{
val += 10;
}
break;
case 'V':
val += 5;
break;
case 'I':
if (i != num.Length - 1)
{
switch (num[i + 1])
{
case 'X':
case 'V':
val -= 1;
break;
case 'I':
val += 1;
break;
default:
return -1;
}
}
else
{
val += 1;
}
break;
default:
return -1;
}
}
return val;
}
public string AsRoman { get; private set; }
public int AsArabic { get; private set; }
}
I really hate that humongous switch
in RomanNumeralToArabic
. Should that be a if
/else if
/else
series, or is there a better way to do it entirely?
Value
method that returns anint
or whatever. This should be a value type (struct), and you shouldn't allow the creation of invalid values (throw during creation). \$\endgroup\$