Roman numeral converter problem

First off I am quite new to programing so the solution is probably quite obvious (ether that or I wrote this program horribly wrong), so bear with me here. I am trying to make a program that converts Roman numerals to Arabic ones and it isn't doing what I thought it would. When I input, for example, "VII" it comes back with something like -2 million!?! BTW the comments are what I think the program does, not necessarily what it actually does. Thanks in advance.

Code:

#include <stdio.h>		/*BEWARE!! THIS DOES NOT WORK YET*/
#include <string.h>
#include <stdlib.h>

void stoupper(char *s) /*definition for string upercase conversion*/
{
for(; *s; s++)
if(('a' <= *s) && (*s <= 'z'))
*s = 'A' + (*s - 'a');
}

char rome[20];
int arab=0;
int lngth;
int count;

int main()
{
	fgets(rome, 20, stdin);		/*gets the roman numeral*/
	stoupper(rome);		/*puts roman numeral in uppercase*/
	lngth=strlen(rome);		/*finds length of numeral*/
	printf("You entered %s\n",rome);
	printf("that is %d charicters long.\n", lngth-1);
	int buf[lngth];		/*for adding up numbers that equal eachother*/
	int lyr[lngth];		/*for the final tally*/
	for(count=0; count<lngth; count++)	/*puts a number value to each letter*/
	{
		switch(rome[count])
		{
			case 'M':
				buf[count]=1000;
				break;
			case 'D':
				buf[count]=500;
				break;
			case 'C':
				buf[count]=100;
				break;
			case 'L':
				buf[count]=50;
				break;
			case 'X':
				buf[count]=10;
				break;
			case 'V':
				buf[count]=5;
				break;
			case 'I':
				buf[count]=1;
				break;
			default:
			
				break;
		}
	}
	int t=0;
	for(count=0; count<lngth; count++)		/* adds up numbers that are the same and moves them to a new aray*/
	{
		if(buf[count]==buf[count+1]) 
		{
			lyr[t]=buf[count]*2;
			t++;
			count++;
		}
		if(buf[count]==buf[count+1] && buf[count]==buf[count+2])
		{
			lyr[t]=buf[count]*3;
			t++;
			count=count+2;
		}
		else
		{
			lyr[t]=buf[count];
			t++;
		}
	}
	for(count=0; count<=t; count++)		/* finds groups that subtract from the total */
	{
		if(lyr[count]<lyr[count+1])
		{
			lyr[count]=lyr[count]*-1;
		}
	}
	for(count=0; count<t; count++)		/*gets final sum */
	{
		arab=arab+lyr[count];
	}
	printf("In arabic:-%d", arab);
}

I don't know much about arabic numerals, but I can tell you this code has some buffer overflows:

Code:

if(buf[count]==buf[count+1] && buf[count]==buf[count+2])
{
	lyr[t]=buf[count]*3;
	t++;
	count=count+2;
}

You are reading past the end of the buf array.

Also, without knowing what you are trying to accomplish, I can almost guarantee you that the for loop which "adds up numbers that are the same and moves them to a new array" is incorrect.

What is your expected output? What do you want to convert "VII" to?

Originally Posted by bithub

I don't know much about arabic numerals

They didn't come from Florida

@the OP: this may be more complicated than you think...is this an assignment, or something you chose to do while learning?

Anyway, it is a classic "parsing" exercise. That approach breaks the numeral down into expressions, the same way you would break:

7+2*5

into 7 and 2*5. Then you add the value of the expressions together. You probably don't quite have to do that, but keep this in mind.

You have two kinds of expressions in roman numerals:

1) groups of descending or equal value, eg, VII, XX, III
2) groups of ascending value, eg. IV, XC, LM.

You seem to have focussed on something that will happen by default -- adding up numbers that are the same.

What you should be concerned with is when the next character is less, in which case the first character modifies the second. Fortunately, those combinations are never more than 2 characters with roman numerals (unlike math expressions).

Iterate thru the string with a ptr until the ptr == '\0'. Each iteration, evaluate the next character, which would be ptr[1]. If it is <=, add ptr[0] to the total (BUT NOT ptr[1]). If not, subtract ptr[0] from ptr[1], add that AND advance the pointer an extra position, so that it starts the next iteration at (what would be) ptr[2], rather than ptr[1] (presuming we move forward at least one every time).

There is a complication: if this is an assignment and you need to verify input, be aware there are things that look like roman numerals that will pass all this, altho they actually are not, eg.

mmcmlxiixxlmmc

I think that requires the parser approach to solve.

I don't know much about arabic numerals, but I can tell you this code has some buffer overflows:
Code:

if(buf[count]==buf[count+1] && buf[count]==buf[count+2])
{
lyr[t]=buf[count]*3;
t++;
count=count+2;
}

You are reading past the end of the buf array.

Also, without knowing what you are trying to accomplish, I can almost guarantee you that the for loop which "adds up numbers that are the same and moves them to a new array" is incorrect.

What is your expected output? What do you want to convert "VII" to?
__________________
bit∙hub [bit-huhb] n. A source and destination for information.

My goal is to convert roman numerals to arabic(pretty sure they're the ones we use ie. 123..etc.) So I'm trying to convert VII to 7. I'm guessing that the overflow is when it tries to read "count+1" or "count+2" when it is at the end of the array and there is no "count+1 or 2"? Sorry for my ignorance. I'm not sure I see the "adds up numbers that are the same and moves them to a new array" problems. It's supposed to see, for example, in VII that "II" are both 1 so "II" is 2, so it treats "VII" as 5 and 2 instead of 5, 1, and 1. (supposed to make them easier to add up)

MK27,
Thanks! It looks like I was over complicating the program. =\

Originally Posted by 7heTexanRebel

My goal is to convert roman numerals to arabic(pretty sure they're the ones we use ie. 123..etc.) So I'm trying to convert VII to 7. I'm guessing that the overflow is when it tries to read "count+1" or "count+2" when it is at the end of the array and there is no "count+1 or 2"? Sorry for my ignorance. I'm not sure I see the "adds up numbers that are the same and moves them to a new array" problems. It's supposed to see, for example, in VII that "II" are both 1 so "II" is 2, so it treats "VII" as 5 and 2 instead of 5, 1, and 1. (supposed to make them easier to add up)

MK27,
Thanks! It looks like I was over complicating the program. =\

Our numbers are indeed arabic numerals. The Greeks (and others) extended geometry significantly, the Arab scholars did the same with algebra, including our number representations.

But that won't help your program.

I like the switch statement - well done on that. Your programs lacks any logic to handle the subtraction needed when a smaller roman numeral, is directly to the left of, a larger roman numeral: e.g.: IV, IX, XD, etc.

As MK27 pointed out, you only need to check ONE numeral to the right of each digit, to see if the subtraction should be done. The amount you should subtract is the lower (first if you read it from left to right) numeral: e.g. IX, adds up to "11", but you now need to subtract 1 from that, because the I is immediately to the left of the larger valued X. Bringing your total to 10 - but you also (wrongly), have added I to the X initially, which needs to be corrected, so subtract one more I value, bringing the total to 9.

So IX = 11 - (2*I)