Roman Numeral troubles....

Im very new to C, only been doing it a few days, so no doubt I'm making this more complex than it needs to be, but im trying to write a program which will print the argument (given as an int) in roman numeral form. I thought Id just try and write something, not really knowing how to go about this, and got this:

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>

int checkint(char s[]) //Checks if string is purely numerical
{
	int t = 1; // Set t=1
	int length = strlen(s); // Define string length
	for (int i = 0; i < length; i++) // For each character
	{
		if (s[i] >= '0' && s[i] <= '9'); // If numerical, then do nothing
		else t = 0; //If not, then set t=0
	}
	return t;
}

static void roman(int n)
{
	char r[6];
	r[0] = 'M';
	r[1] = 'D';
	r[2] = 'C';
	r[3] = 'L';
	r[4] = 'X';
	r[5] = 'V';
	r[6] = 'I';
	
	int t[6];
	t[0] = 1000;
	t[1] = 500;
	t[2] = 100;
	t[3] = 50;
	t[4] = 10;
	t[5] = 5;
	t[6] = 1;
	
	int i[6];
	for (int a = 0; a < 7; a++)
	{
		if (n > t[a])
		{
			i[a] = n / t[a];
			n = n - i[a]*t[a];
			
		}
		else;
		int b = 0;
		while (b < i[a])
		{
			printf("%c", r[a]);
			b++;
		}
	} 
	
	printf("\n");	
}

int main(int argc, char *argv[])
{
    if(argc < 2) // Error is number of arguments is less than 2
	{
		fprintf(stderr, "%s: program expected 1 or more arguments, but received %d\n", argv[0], argc-1);
		exit(EXIT_FAILURE);
    }
    else 
	{
		// Loop for each argument passed to the program
		for(int i = 1; i < argc; i++) 
		{
			if (!checkint(argv[i])) // Check arguments provided are numerical
			{ // If not, then exit and give error message
				fprintf(stderr, "%s: program expected positive numerical arguments\n", argv[0]);
				exit(EXIT_FAILURE);
			}
			else
			{
				int n = atoi(argv[i]); // Argument as integer
				roman(n); // Call roman function
			/*	printf("%d as a Roman Numeral is %s\n", n, s); // Return string */
			}
		}		
		// Exit indicating success
		exit(EXIT_SUCCESS);
	}
    return 0;
}

Which spits out random results not what theyre meant to be. Obviously even if this worked, it still isn't taking into account cases like 900, 90, 9, 400, 4, 40 ect. which all use subtraction in their roman numeral form. Still no idea how to implement that part of it.
Help would be nice, im totally stuck on this =/

Just guessing here but I always get messed up with arrays so I could be wrong.

Code:

	char r[6];
	r[0] = 'M';
	r[1] = 'D';
	r[2] = 'C';
	r[3] = 'L';
	r[4] = 'X';
	r[5] = 'V';
	r[6] = 'I';

You have declared an array with 6 spaces in in but you have put 7 chatacters into it.
0 to 6 = 7 not 6!!




That would screw you up a bit, I think?
I expect you are overwritging something, change char r[6] to char r[7]
same for int t[6];
Apart form that it looks like a nice program.

Incidently I nearly always make my arrays bigger then they need to be by at least 1 so I don't end up pulling my hair out.

As for the next bit you could just check if the number was 900, 90, 9, 400, 4, 40 etc and just print the result from a table.

so you have

Code:

special_numbers[10]={4,9,90,900}
special strings[10]={"IV", "IX",.......

Then print the string from the matching number, easy , well maybe, it might be more complicated I am not 100% of
roman numerals.

All your arrays in roman() are one element too short.

You also need to make your result larger as well, because some results are much longer than the decimal equivalents.
eg
1888 = MDCCCLXXXVIII

lol thanks for that, Im always confusing myself with arrays and counting from 0 aswell....
Also I fixed up the printf and while statement, which were meant to be within the if n > t[a] statement, so that it now semi-works. If i give it 5563 for example, it returns MMMMMDLXIII, which is correct. Now I've got the issue of 4's and 9's, anyone got any ideas on how to deal with them ?

Maybe an if to check for 4's and 9's, but then what ? :S


Salem: what do you mean make my result longer ?
I dont really have a result just a looping printf statement at this stage..

You need to recheck the logic using a debugger.
Take a number less than 1000, what would be the initial value of i[a] when execution
flows to this part of the code?

Code:

	int i[6];
	for (int a = 0; a < 7; a++)
	{
		if (n > t[a])
		{
			i[a] = n / t[a];
			n = n - i[a]*t[a];
			
		}
		else;
		int b = 0;
		while (b < i[a])
		{
			printf("%c", r[a]);
			b++;
		}
	}

You have declared an array with 6 spaces in in but you have put 7 chatacters into it.
0 to 6 = 7 not 6!!

Actually there are enough spaces for 7 elements already, but the OP needs to allocate one more for the null terminating character '\0'.

Thanks all for your help, I've condensed the decleration of the arrays down a bit, and added in support for 4's and 9's, which I realised actually fits in with the current way the program runs, only the arrays needed to be extended. Program now works almost perfectly, except when you put in 9 for instance, it prints the "IX", but then follows it with a "V", asif its continuing some loop somewhere. For the life of me Ive no idea why.... ??

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>

int checkint(char s[]) //Checks if string is purely numerical
{
	int t = 1; // Set t=1
	int length = strlen(s); // Define string length
	for (int i = 0; i < length; i++) // For each character
	{
		if (s[i] >= '0' && s[i] <= '9'); // If numerical, then do nothing
		else t = 0; //If not, then set t=0
	}
	return t;
}

static void roman(int n)
{
	char r[13][2] = {"M","CM","D","CD","C","XC","L","XL","X","IX","V","IV","I"};
	int t[13] = {1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1};
	int i[13];
	for (int a = 0; a < 13; a++)
	{
		if (n >= t[a])
		{
			i[a] = n / t[a];
			n = n - i[a]*t[a];
			
			for (int b = 0; b < i[a]; b++)
				printf("%s", r[a]);
		}
		else;
	} 
	
	printf("\n");	
}

int main(int argc, char *argv[])
{
    if(argc < 2) // Error is number of arguments is less than 2
	{
		fprintf(stderr, "%s: program expected 1 or more arguments, but received %d\n", argv[0], argc-1);
		exit(EXIT_FAILURE);
    }
    else 
	{
		// Loop for each argument passed to the program
		for(int i = 1; i < argc; i++) 
		{
			if (!checkint(argv[i])) // Check arguments provided are numerical
			{ // If not, then exit and give error message
				fprintf(stderr, "%s: program expected positive numerical arguments\n", argv[0]);
				exit(EXIT_FAILURE);
			}
			else
			{
				int n = atoi(argv[i]); // Argument as integer
				roman(n); // Call roman function
			}
		}		
		// Exit indicating success
		exit(EXIT_SUCCESS);
	}
    return 0;
}

Code:

		if (s[i] >= '0' && s[i] <= '9'); // If numerical, then do nothing
		else t = 0; //If not, then set t=0

Yeuch! How about re-writing that so that you do not have a "do nothing". Also, once you have found ONE non-numerical, you can return then and there - no point in checking the rest of the string.

--
Mats

I think there is a function, I remember one call isalpha() there is a isdigit() one too but it's
easy enough to write the code anyway.
The program looks OK to me, at least I can't see how it would work any different for IX.
Sure you didn't type in 95?

Originally Posted by esbo

I think there is a function, I remember one call isalpha() there is a isdigit() one too but it's
easy enough to write the code anyway.
The program looks OK to me, at least I can't see how it would work any different for IX.

Yes, isalpha() and isdigit() can be used for this purpose - but I was actually more intending to complain about the formation of the if-statement, not the actual construction inside the if-statement.

However, seeing as the code already includes ctype.h, it makes sense to use the "ctype.h" function called isdigit(), which tells you if a character is a digit, instead of comparing with 0..9.

--
Mats

Thanks I didnt know about the isdigit() function, so Ive changed that over and fixed the returning as per matsp's advice. The roman function still isn't working properly though, for anything with a 4 and a 9 in it, it prints the correct characters followed by another.
For example 49 (which should be XLIX) prints as XLXIXV. It might be a logic error I dunno...

nvm, fixed.
I've highlighted the error, incase anyone is interested, made the same mistake twice lol, forgot to allow an extra character in the each string of the array for the terminating character

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>

int checkint(char s[]) //Checks if string is purely numerical
{
	int length = strlen(s); // Define string length
	for (int i = 0; i < length; i++) // For each character
	{
		if (isdigit(s[i])); // If numerical, then do nothing
		else return 0; //If not, then return 0
	}
	return 1;
}

static void roman(int n)
{
	char r[13][3] = {"M","CM","D","CD","C","XC","L","XL","X","IX","V","IV","I"};
	int t[13] = {1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1};
	int i[13];
	for (int a = 0; a < 13; a++)
	{
		if (n >= t[a])
		{
			i[a] = n / t[a];
			n = n - i[a]*t[a];
			
			for (int b = 0; b < i[a]; b++)
				printf("%s", r[a]);
		}
		else;
	} 
	
	printf("\n");	
}

int main(int argc, char *argv[])
{
    if(argc < 2) // Error is number of arguments is less than 2
	{
		fprintf(stderr, "%s: program expected 1 or more arguments, but received %d\n", argv[0], argc-1);
		exit(EXIT_FAILURE);
    }
    else 
	{
		// Loop for each argument passed to the program
		for(int i = 1; i < argc; i++) 
		{
			if (!checkint(argv[i])) // Check arguments provided are numerical
			{ // If not, then exit and give error message
				fprintf(stderr, "%s: program expected positive numerical arguments\n", argv[0]);
				exit(EXIT_FAILURE);
			}
			else
			{
				int n = atoi(argv[i]); // Argument as integer
				roman(n); // Call roman function
			}
		}		
		// Exit indicating success
		exit(EXIT_SUCCESS);
	}
    return 0;
}

Well spotted, you can probably see why I always tend to make my arrays larger than they need to be, it can save a whole lot of trouble. why declare 2 bytes when you can delcare 30?
Such bugs can be really hard to track down, imagine if that had been a 1000 line program!
Not quite the same problem except in that you have to have a space for the 0 in a string.

It might have been easier to spot if V had been a 2 character string, say JJ, then it would have put
IXJJIVI

Originally Posted by esbo

Well spotted, you can probably see why I always tend to make my arrays larger than they need to be, it can save a whole lot of trouble. why declare 2 bytes when you can delcare 30?
Such bugs can be really hard to track down, imagine if that had been a 1000 line program!
Not quite the same problem except in that you have to have a space for the 0 in a string.

It might have been easier to spot if V had been a 2 character string, say JJ, then it would have put
IXJJIVI

But constantly oversizing arrays can also lead to problems - particularly if you have MANY of those oversize arrays. In this particular case, it's not many, so it's no big deal - but it's very easy to get carried away and get enormous memory usage from this type of thing.

--
Mats

Originally Posted by matsp

But constantly oversizing arrays can also lead to problems - particularly if you have MANY of those oversize arrays. In this particular case, it's not many, so it's no big deal - but it's very easy to get carried away and get enormous memory usage from this type of thing.

--
Mats

True but it's fairly easy to resize them if problems arise, certainly less troublesome than if
it results in some obscure bug which emerges in the code some time later.
Anway I have 1.24 gigs of memory, it's not *that* long ago that my entire file system was 2 gigs.I generally err on the side of caution and make my arrays one element longer and use sizeable input buffers much bigger than the expected input. It has never cause me a problem that I can recall by making storage too big, however making it just one byte too small can cause you no end of trouble!

Although this wasn't the dimension you were having touble with, it might be better if you don't specify the size at all, where possible

Code:

char r[][3] = {"M","CM","D","CD","C","XC","L","XL","X","IX","V","IV","I"};
int t[] = {1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1};
int i[sizeof(t)/sizeof(t[0])];

for (int a = 0; a < sizeof(t)/sizeof(t[0]); a++)