Thread: Roman Numeral troubles....

  1. #1
    Registered User
    Join Date
    Jan 2009
    Posts
    15

    Unhappy 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 =/

  2. #2
    Fountain of knowledge.
    Join Date
    May 2006
    Posts
    794
    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.
    Last edited by esbo; 01-15-2009 at 12:13 AM.

  3. #3
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    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
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  4. #4
    Registered User
    Join Date
    Jan 2009
    Posts
    15
    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..

  5. #5
    and the hat of copycat stevesmithx's Avatar
    Join Date
    Sep 2007
    Posts
    587
    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'.
    Not everything that can be counted counts, and not everything that counts can be counted
    - Albert Einstein.


    No programming language is perfect. There is not even a single best language; there are only languages well suited or perhaps poorly suited for particular purposes.
    - Herbert Mayer

  6. #6
    Registered User
    Join Date
    Jan 2009
    Posts
    15
    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;
    }

  7. #7
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  8. #8
    Fountain of knowledge.
    Join Date
    May 2006
    Posts
    794
    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?
    Last edited by esbo; 01-15-2009 at 04:22 AM.

  9. #9
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by esbo View Post
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  10. #10
    Registered User
    Join Date
    Jan 2009
    Posts
    15
    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...

  11. #11
    Registered User
    Join Date
    Jan 2009
    Posts
    15
    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;
    }

  12. #12
    Fountain of knowledge.
    Join Date
    May 2006
    Posts
    794
    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

  13. #13
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by esbo View Post
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  14. #14
    Fountain of knowledge.
    Join Date
    May 2006
    Posts
    794
    Quote Originally Posted by matsp View Post
    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!

  15. #15
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    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++)
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. roman numeral conversion
    By clarky101 in forum C Programming
    Replies: 8
    Last Post: 12-09-2007, 07:13 PM
  2. Roman Numeral...T_T
    By only1st in forum C++ Programming
    Replies: 12
    Last Post: 04-12-2007, 10:06 AM
  3. Roman Numeral Convertor
    By mike1000 in forum C Programming
    Replies: 2
    Last Post: 11-24-2005, 02:53 PM
  4. Roman Numeral Calculator
    By The Brain in forum C++ Programming
    Replies: 3
    Last Post: 04-13-2005, 06:23 PM
  5. implicit declatation of function 'int toupper(...)'
    By Intimd8r in forum C Programming
    Replies: 3
    Last Post: 10-01-2001, 02:43 PM