• 0

roman numeral calculator


Question

I'm trying to build a roman numeral calculator....I'm not quite sure what happened with my code, it was working just fine and now it wont do what it's supposed to do.  I've gone over line by line with a fine toothed comb and cannot seem to find where it's going wonky.  Here is what I have

/* Amanda Strote
	Roman Calculator
	CISP 360
	*/
#include <stdio.h>
#include <conio.h>
#include <stdlib.h> //rand and srand is in here
#include <time.h> //contains time structure
#pragma warning (disable:4996) //turns off stdio function warnings

char num1, num2;  //users 1st and 2nd entered roman numerals
int opchar; //operator characters
int yesno; //users answer to running again
int ch1, ch2;
int ans;
int opOK; //validation for the entered operator character
int again;
time_t t; //not needed for this program

void main()
{
	printf("Welcome to the Roman Calculator.\n"); //welcome message
	//main processing loop
	do{
		ch1 = ch2 = num1 = num2 = ans = 0; //initialized to zero
		again = 2;
		printf("Please enter your first number: ");
		do{
		//get user input
		num1 = getche();
		switch(num1)
		{
		case 'M':
		case 'm':
			ch1 += 1000;
			break;
		case 'D':
		case 'd':
			ch1 += 500;
			break;
		case 'C':
		case 'c':
			ch1 += 100;
			break;
		case 'L':
		case 'l':
			ch1 += 50;
			break;
		case 'X':
		case 'x':
			ch1 += 10;
			break;
		case 'V':
		case 'v':
			ch1 += 5;
			break;
		case 'I':
		case 'i':
			ch1 =+ 1;
			break;
		default:
			//invalid entries will be ignored 
			break;
		}
	}while(num1 != 'Q' && num1 !='q'); //ends loop for 1st #
	printf("\nFirst number = %d\n\n", ch1);

	//users 2nd number
	printf("Please enter your second number: ");
	do{
		//gets user input
		num2 = getche();
		switch(num2)
		{
		case 'M':
		case 'm':
			ch1 += 1000;
			break;
		case 'D':
		case 'd':
			ch1 += 500;
			break;
		case 'C':
		case 'c':
			ch1 += 100;
			break;
		case 'L':
		case 'l':
			ch1 += 50;
			break;
		case 'X':
		case 'x':
			ch1 += 10;
			break;
		case 'V':
		case 'v':
			ch1 += 5;
			break;
		case 'I':
		case 'i':
			ch1 =+ 1;
			break;
		default:
			//invalid entries will be ignored 
			break;
		}
	}while(num2 != 'Q' && num2 !='q'); //ends loop for 2nd #

	//display 2nd number in roman numerals
	printf("\n Second number = %d\n", ch2);

	do{ //loop that looks for valid operators
		printf("\nPlease select and operation (*, /, +, -, %): ");
		opchar = getche(); //get user's input

		//checks for valid operation and perform's operations
		switch(opchar)
		{
		case '+':
			ans = ch1 + ch2;
			opOK = true;
			break;
		case '-':
			ans = ch1 - ch2;
			opOK = true;
			break;
		case '*':
			ans = ch1 * ch2;
			opOK = true;
			break;
		case '/':
			if(num2 ==0) //catches undefined answers
			{
				printf("\nCannot divide by zero.  Select another operation.\n");
				opOK = false;
				break;
			}
			else
			{
				ans = ch1 / ch2;
				opOK = true;
				break;
			}
defalut:  //catches improper input
			printf("Unidentified operation.  Select another operation.\n");
			opOK = false;
		}
	}while(opOK != true); //exits the loop when opOK becomes true

	//convert and display roman numerals
	printf("\n\nThe answer is %d\nRoman Numeral answer is ", ans);
	if(ans < 0)
	{
		printf("-");
		ans *= -1;
	}
	while(ans >= 1000)
	{
		printf("M");
		ans -= 1000;
	}
	while(ans >= 500)
	{
		printf("D");
		ans -= 500;
	}
	while(ans >= 100)
	{
		printf("C");
		ans -= 100;
	}
	while(ans >= 50)
	{
		printf("L");
			ans -= 50;
	}
	while(ans >= 10)
	{
		printf("X");
		ans -= 10;
	}
	while(ans>= 5)
	{
		printf("V");
		ans -= 5;
	}
	while(ans >= 1)
	{
		printf("I");
		ans -= 1;
	}
	//prompts the user if they would like to run again
	printf("\nDo you want to play again? (y)es or (n)o?");
	do{
		yesno=getche();
		switch(yesno)
		{
		case 'Y':
		case 'y':
			again = 1;
			break;
		case 'N':
		case 'n':
			again = 0;
			break;
		default:
			printf("\nNot a valid entry, please select (y)es or (n)o. ");
		}
	}while(again ==2); //exits loop when the user enters y or n
	printf("\n\n"); //line break
	}while(again==1); //ends main loop
	printf("\nBye-bye!\n\n");
	}










Here is what it's doing:

 

Welcome to the Roman Calculator.
Please enter your first number: 5

 

 

 

And here is what its supposed to do:

 

Welcome to Roman Calculator.  Hail Caesar!

 

Enter first number: MMXI

The first number is : 2011

Enter second number: VIII

The second number is : 8

Enter operation (* / + - ): +

Answer: 2019

In Roman: MMXVIIII

 

Press Q to quit, any other key to continue.

Enter first number: XXII

The first number is : 22

Enter second number: VII

The second number is : 7

Enter operation (* / + - ): /

Answer: 3

In Roman: III

 

Press Q to quit, any other key to continue. Thanks for playing!

Press any key to continue . . .

 

 

 

Someone said I was supposed to have had a \r in there, but I'm not sure where it would go to be honest.

Link to comment
Share on other sites

14 answers to this question

Recommended Posts

  • 0

I'm trying to build a roman numeral calculator....I'm not quite sure what happened with my code, it was working just fine and now it wont do what it's supposed to do.  I've gone over line by line with a fine toothed comb and cannot seem to find where it's going wonky.  

Did you step through it in the debugger? If you're using visual studio, it's as simple as hitting F10 and putting your cursor over variables to see their values. If you learn to use the debugger, you'll never say "I don't know what my program is doing" again.

Link to comment
Share on other sites

  • 0

I'm not trying to sound harsh, but does your program compile? There are a few problems I noticed off-hand that should prevent compilation. Even if they do not, they are probably causing problems. First, the label of the default case in the loop where you ask the user to specify an operation is spelled wrong. It should be "default", not "defalut". Second, you are assigning true and false values to an integer (opOK) in that same switch. Not only is that undefined behavior in C++ (where bool is a valid type), but there is no such identifier as true or false in C. You should instead assign opOK for false and 1 for true.

 

There are also a couple of obvious mistakes that should not result in compiler errors (although most decent C compilers should warn about them), but will probably result in runtime errors. First, you specified % as a valid operation, but forgot to escape it in your printf() statement. Since printf() uses the percent sign as its escape character, to actually print that character you need two of them (so printf( "%%" ) will result in a single percent sign being printed). Additionally, although your dialog says that % is a valid operation, it is not handled in the following switch case.

 

Furthermore your code is difficult to follow for several reasons. It would greatly aid readability and probably make your program easier to debug, like Asik suggested, if you make the changes I am about to suggest. One of the first things I noticed when I read your program is an abundance of code duplication. Especially considering how similar your Roman-Numeral-to-Decimal and Decimal-to-Roman-Numeral conversions are, it would probably be best to write functions to take care of that processing. In addition to eliminating most of your code duplication, it would make the program much easier to read. On a related note, you could remove the upper and lower case characters and just use one or the other by using either the C standard library function tolower() or toupper() to arbitrarily convert the Roman Numeral entered by the user to either lower or upper case, respectively.

/*
Convert a single Roman Numeral to its decimal equivalent.

Arguments:
    numeral [in]        Roman Numeral to convert

Return Value:
    -1      The input character is not a valid Roman Number.
    >0      A decimal number!
*/
int roman_to_decimal( char numeral )
{
    switch( tolower( numeral ) )
    {
        case 'm':
            return 1000;
        case 'd':
            return 500;
        case 'c':
            return 100;
        case 'l':
            return 50;
        case 'x':
            return 10;
        case 'v':
            return 5;
        case 'i':
            return 1;
    }
    return -1;
}

/*
Convert a string of Roman Numeral to its decimal equivalent.

Side Effects:
    If this function fails the contents of the decimal must not be relied upon!

Arguments:
    numerals [in]       NULL-terminated string of Roman Numerals to be converted
    decimal [out]       Decimal equivalent of the input string

Return Value:
    -2      An unknown error occurred.
    -1      The input string is not a valid Roman Number.
     0      The input string was converted properly.
*/
int roman_str_to_decimal( const char * numerals, int * decimal )
{
    int n; // Is the number negative?
    int d; // Temporary decimal
    
    if( !numerals ) return -2;
    
    *decimal = 0;
    if( *numerals == '-' )
    {
        n = 1;
        numerals++;
    }
    else
    {
        n = 0;
    }
    
    for( ; *numerals; numerals++ )
    {
        d = roman_to_decimal( *numerals );
        if( d <= 0 ) return -1;
        *decimal += d;
    }
    
    if( n ) *decimal *= -1;
    
    return 0;
}

/*
Convert a decimal number to its equivalent in Roman Numerals.

Side Effects:
    If this function fails the contents of the output array must not be relied
    upon!

Arguments:
    decimal [in]        Decimal number to convert
    numerals [out]      Array the Roman Numerals will be written to
                        This string will be NULL-terminated.
    size [in]           Size of the numerals array

Return Value:
    -2      An unknown error occurred.
    -1      The output array is too small!
    >=0     This is the number of characters written to the array,
            excluding the terminating character.
*/
int decimal_to_roman_str( int decimal, char * numerals, int size )
{
    int n = 0; // Number of characters written to the output array
    
    if( size <= 0 ) return -2;
    
    if( decimal < 0 )
    {
        numerals[n++] = '-';
        decimal *= -1;
    }
    
    while( n < size )
    {
        if( decimal >= 1000 )
        {
            numerals[n++] = 'M';
            decimal -= 1000;
        }
        else if( decimal >= 500 )
        {
            numerals[n++] = 'D';
            decimal -= 500;
        }
        else if( decimal >= 100 )
        {
            numerals[n++] = 'C';
            decimal -= 100;
        }
        else if( decimal >= 50 )
        {
            numerals[n++] = 'L';
            decimal -= 50;
        }
        else if( decimal >= 10 )
        {
            numerals[n++] = 'X';
            decimal -= 10;
        }
        else if( decimal >= 5 )
        {
            numerals[n++] = 'V';
            decimal -= 5;
        }
        else if( decimal >= 1 )
        {
            numerals[n++] = 'I';
            decimal -= 1;
        }
        else
        {
            numerals[n] = '\0';
            break;
        }
    }
    
    if( n == size ) return -1;
    return n;
}

It would also make your program simpler to reduce dead code by removing the unused t variable and time.h and stdlib.h includes. Finally, you would do well to part with your fondness of do {} while() loops. Not only is do {} while() one of the most infrequently used loops in practice, but your excessive use of it makes your code unnecessarily complicated. Your program logic would be much simpler if you used a single while() loop to give the user multiple tries without the unnecessary complication.

  • Like 1
Link to comment
Share on other sites

  • 0

Here's what it's doing:

Welcome to the Roman Calculator.

Please enter your first number: 5

 

And here is what its supposed to do:

Welcome to Roman Calculator.  Hail Caesar!

Enter first number: MMXI

The first number is : 2011

 

You forgot the Hail Caesar! That's why it doesn't work :laugh:

 

But seriously, do you not see anything wrong with your input? (Hint: 5)

Link to comment
Share on other sites

  • 0

Although I didn't catch it yesterday, I think GreenMartian is right; your input looks flawed. Below is the output from a run of a refactored version of your program I wrote last night while I was looking for changes to suggest; maybe it will help.

Welcome to the Roman Calculator.
Please enter your first number: XVIII
First number = 18
Please enter your second number: L
Second number = 50
Please select and operation (*, /, +, -, %): +
Answer: 68 ==> LXVIII
Do you want to play again? (y)es or (n)o? y
Please enter your first number: -CV
First number = -105
Please enter your second number: X
Second number = 10
Please select and operation (*, /, +, -, %): *
Answer: -1050 ==> -ML
Do you want to play again? (y)es or (n)o? n
Bye-bye!
Link to comment
Share on other sites

  • 0

Are you actually hitting "q" when you've finished entering a number? I can build and run your code fine. The calculations don't work, but I'll leave that to you to figure out.

Link to comment
Share on other sites

  • 0

Are you actually hitting "q" when you've finished entering a number? I can build and run your code fine. The calculations don't work, but I'll leave that to you to figure out.

 

You can build it?! After making the obvious portability changes (remove the conio.h include and make main() return an int), I get the following warnings and errors:

$ clang -x c -o roman.bin roman.c.orig
roman.c.orig:34:20: warning: implicit declaration of function 'getche' is invalid in C99 [-Wimplicit-function-declaration]
            num1 = getche();
                   ^
roman.c.orig:63:25: warning: use of unary operator that may be intended as compound assignment (+=)
                    ch1 =+ 1;
                        ^~
roman.c.orig:106:25: warning: use of unary operator that may be intended as compound assignment (+=)
                    ch1 =+ 1;
                        ^~
roman.c.orig:120:65: warning: invalid conversion specifier ')' [-Wformat-invalid-specifier]
            printf("\nPlease select and operation (*, /, +, -, %): ");
                                                               ~^
roman.c.orig:128:28: error: use of undeclared identifier 'true'
                    opOK = true;
                           ^
roman.c.orig:132:28: error: use of undeclared identifier 'true'
                    opOK = true;
                           ^
roman.c.orig:136:28: error: use of undeclared identifier 'true'
                    opOK = true;
                           ^
roman.c.orig:142:32: error: use of undeclared identifier 'false'
                        opOK = false;
                               ^
roman.c.orig:148:32: error: use of undeclared identifier 'true'
                        opOK = true;
                               ^
roman.c.orig:153:28: error: use of undeclared identifier 'false'
                    opOK = false;
                           ^
roman.c.orig:155:25: error: use of undeclared identifier 'true'
        } while(opOK != true); //exits the loop when opOK becomes true
                        ^
4 warnings and 7 errors generated.
Link to comment
Share on other sites

  • 0

It builds on GCC okay after I hacked out the Windows-specific stuff, but yeah it does compile...

karl@HERA (~/Development/roman) -> gcc roman.c
karl@HERA (~/Development/roman) -> 
Patch attached :p

roman.c.patch.txt

Link to comment
Share on other sites

  • 0

You can build it?! After making the obvious portability changes (remove the conio.h include and make main() return an int), I get the following warnings and errors:

It is likely he compiles as C++. You have to explicitely name your files ".c" or force the project to compile as C in VS, the default is .cpp files that compile as C++. That would explain why "true" and "false" are valid identifiers.

Link to comment
Share on other sites

  • 0

It builds on GCC okay after I hacked out the Windows-specific stuff, but yeah it does compile...

 

That patch is more extensive than my modifications. Although, GCC 4.7 only complains about true and false not being identifiers, not all the stuff Clang 3.0 complained about. This reminds me why I prefer Clang.

 

Also, I had no idea that stdbool.h existed! Thanks for the tip.

Link to comment
Share on other sites

  • 0

That patch is more extensive than my modifications.

That's only a lot because I copy/pasted someone's code to provide Linux equivalents of the conio.h functions :p

Link to comment
Share on other sites

  • 0

This reminds me why I prefer Clang.

+1. Its error reporting was enough to convert me over for most of my projects back then, even before it reached feature parity with gcc a couple years ago.

 

Then again, that's why competition is good. GCC has warmed up to the idea and is starting to catch up now http://gcc.gnu.org/wiki/ClangDiagnosticsComparison

But we digress..

 

Part of the trickiness of doing roman numerals are the "one-off" rule. E.g. IV=4, IX=9, XL=40, XC=90, etc which usually caught most people off guard.

As the classic saying goes, "These Romans are crazy" -Asterix

Link to comment
Share on other sites

This topic is now closed to further replies.
  • Recently Browsing   0 members

    • No registered users viewing this page.