Roman Numerals

[nothing3]

I am attempting to create a program that will convert roman numerals to decimals and decimals to roman numerals. I have a code that works for the most part but, the answer it is achieving for the first and second test numeral are wrong. instead of coming up with the numbers 1114, 359 and 1666 it comes up with 11, 6 and 1666. Any help would be appreciated.

#include<string>
#include<iostream>

using namespace std;

class romanType
{

public :
romanType( string = "" );
void setRoman( string );
void convertToDecimal();
void printRoman();
void printDecimal();

private:
string roman;
int decimal;
};

romanType::romanType( string myRoman )
{
roman = myRoman;
decimal = 0;
}

void romanType::setRoman( string myRoman )
{
roman = myRoman;
decimal = 0;
}

void romanType::convertToDecimal()
{
char romans[7] = { 'M', 'D', 'C', 'L', 'X', 'V', 'I'};
int decimals[ 7 ] = { 1000, 500, 100, 50, 10, 5, 1 };
int j, pos;
size_t len = roman.length();

for ( unsigned int i = 0; i < len - 1; i++ )
{

for ( pos = 0; pos < 7; pos++ )
if ( roman.at( i ) == romans[ pos ] )
break;

if ( pos < 7 )
{
for ( j = 0; j < pos; j++ )
if ( roman.at( i + 1 ) == romans[ j ] )
break;

if ( j == pos )
decimal += decimals[ pos ];
else
decimal = decimals[ pos ];
}
}


for ( j = 0; j < 7; j++ )
if ( roman.at( len - 1 ) == romans[ j ] )
break;

decimal += decimals[ j ];

}

void romanType::printRoman()
{
cout << "\n\n\tThe roman numeral is " << roman;
}

void romanType::printDecimal()
{
cout << "\n\tThe decimal equivalent of the "
<< "given roman numeral is " << decimal;
}



int main()
{

cout << "\n\n\tProgram that convert Roman Numeral"
<< " into decimal form.";

romanType r;
string rns[ 3 ] = { "CCCLIX", "MCXIV", "MDCLXVI" };

for ( int i = 0; i < 3; i++ )
{
r.setRoman( rns[ i ] );
r.convertToDecimal();
r.printRoman();
r.printDecimal();
}

cout << "\n\n\t";
system( "pause" );
return 0;
}

[Maniax]

Please use code tags :>

Here's an easy way to do it, I hope i didn't spoil the "fun"

enum romans {I = 1, V = 5, X = 10, L = 50, C = 100, D = 500, M = 1000};

void romanType::convertToDecimal()
{
	for (int i = 0; i < roman.size(); i++ )  //you could use roman.length() if you like
	{
		switch (roman[i]) //roman.at(i) is just a more secure way to do it
		{
			case 'M': decimal += M; break;
			case 'D': decimal += D; break;
			case 'C': decimal += C; break;
			case 'L': decimal += L; break;
			case 'X': decimal += X; break;
			case 'V': decimal += V; break;
			case 'I':
				if (roman[i + 1] != 'I' && i + 1 != roman.size())
					decimal-=1;
				else
					decimal+=1;
				break;
		}
	}
}

What you did was check if the first letter was M, the second letter D and so on, thats why the last number was correct.

[nothing3]

thank you for your response... what do you mean by using code tags?

[andywestken]

@Maniax

How well does your function handle XCIX (99) or MCMLXIII (1963)?

[ToniAz]

@Maniax

Mr. andywestken is right!


@nothing3

You need to use a single Roman numeral to decimal converter and iterate, character by character, over the string and add everything up.

And code tags are the pointy brackets on the right side.

[tntxtnt]

here's a quick and simple way to do it: (too many "magic" numbers though)

int roman2decimal(const string& roman) //assume roman is uppercase
{
   int result = 0;
   const int LEN = roman.length();

   for (int i = 0; i < LEN; ++i)
   {
      if (roman[i] == 'I' && i != LEN-1)
      {
         if      (roman[i+1] == 'V') { result +=    4; ++i; } //IV leap V char
         else if (roman[i+1] == 'X') { result +=    9; ++i; } //IX leap X char
         else                        { result +=    1;      }
      }
      else if (roman[i] == 'X' && i != LEN-1)
      {
         if      (roman[i+1] == 'L') { result +=   40; ++i; } //XL leap L char
         else if (roman[i+1] == 'C') { result +=   90; ++i; } //XC leap C char
         else                        { result +=   10;      }
      }
      else if (roman[i] == 'C' && i != LEN-1)
      {
         if      (roman[i+1] == 'D') { result +=  400; ++i; } //CD leap D char
         else if (roman[i+1] == 'M') { result +=  900; ++i; } //CM leap M char
         else                        { result +=  100;      }
      }
      else if (roman[i] == 'I')      { result +=    1; }
      else if (roman[i] == 'V')      { result +=    5; }
      else if (roman[i] == 'X')      { result +=   10; }
      else if (roman[i] == 'L')      { result +=   50; }
      else if (roman[i] == 'C')      { result +=  100; }
      else if (roman[i] == 'D')      { result +=  500; }
      else if (roman[i] == 'M')      { result += 1000; }
   }
   return result;
}

[eypros]

You can also modify @Maniax's code:

void romanType::convertToDecimal()
{
	for (int i = 0; i < roman.size(); i++ )  //you could use roman.length() if you like
	{
		switch (roman[i]) //roman.at(i) is just a more secure way to do it
		{
			case 'M': decimal += M; break;
			case 'D': 
                                 if(i + 1 != roman.size() &&roman[i+1] == 'M')
                                        decimal -= D;
                                else
                                        decimal += D; break;
			case 'C': 
                                 if(i + 1 != roman.size() && roman[i+1] == 'M' || roman[i+1] == 'D')
                                        decimal -= C;
                                else
                                        decimal += C; break;
			case 'L': decimal += L; break;
                                     etc...
			case 'X': decimal += X; break;
                                     etc...
			case 'V': decimal += V; break;
                                     etc...
			case 'I':
				if ( i + 1 != roman.size() && roman[i + 1] != 'I')
					decimal-=1;
				else
					decimal+=1;
				break;
		}
	}
}

which looks (to me at last) more intuitive as you just check if next digital is greater than current one and if so it's detracted instead of added to the Roman number.

Also I changed the order of boundary check since C++ used short-circuited calculation.

[Maniax]

@andywestken

I didn't know this XC was valid, I only knew about IV IX and so on...

well you just copy + past the code at case 'I': for all other ones