Thread: Reading characters from a string and assigning them to a single char???

  1. #16
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Code:
    void initialise (char []);
    void initialize (int []);
    Not allowed because they have the same name but different parameters. Valid in C++, but not C.

  2. #17
    Registered User
    Join Date
    Nov 2007
    Posts
    12
    Also I need a file read in or input from keyboard - and could not figure out how to do it without duplicating the code with a massive if???

  3. #18
    Registered User
    Join Date
    Nov 2007
    Posts
    12
    Quote Originally Posted by Elysia View Post
    Code:
    void initialise (char []);
    void initialize (int []);
    Not allowed because they have the same name but different parameters. Valid in C++, but not C.
    I spelt one with an 's' and one with a 'z'...

  4. #19
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Not sure what you're hinting at?

  5. #20
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by DonGenaro View Post
    I spelt one with an 's' and one with a 'z'...
    I see, very subtle difference there. I don't think that's a good idea at all. Consider renaming them.

  6. #21
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by DonGenaro View Post
    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    #include <ctype.h>
    
    enum count {CountM, CountD, CountC, CountL, CountX, CountV, CountI, CountCM, CountCD, CountXC, CountXL, CountIX, CountIV};
    
    char LETTERS[16]; /* Don't use global variables! */
    
    /* Consider renaming. */
    void initialise (char []);
    void initialize (int []);
    /* All functions that takes no params should take void in C */
    void restart ();	
    void readkeyboard();					// reads the string from the keyboard 
    void ask ()	;					// asks for input type
    void choice();						// choose between file or keyboard
    
    
    
    /* Use ZeroMemory or memset. Much easier and will alow you to avoid these superflous functions */
    void initialize (int tab[])
    {
    	int i;
    	for (i=0; i<=15; ++i)
    		tab[i] = 0;
    }
    
    void initialise (char tab[])
    {
    	int i;
    	for (i=0; i<=15; ++i)
    		tab[i] = 0;
    }
    
    
    void restart ()			// RESTARTS THE WHOLE PROGRAMME
    {
    	int yn; /* You want to read a char, not an int */
    	printf("Do you wish to try again? Enter Y to continue \n(else enter anything else to quit): ");
    	scanf("%i", &yn); /* Should be scanf("%c", &yn); because you want to read a char, not an int */
    
    	if(tolower(yn) == 'y')
    	{
    		ask (); 
    		choice();
    	}
    }
    
    
    void readkeyboard ()			// FOR KEYBOARD MODE ASK FOR NUMERALS AND COPY INTO "LETTERS"
    {									//and change string to upper case	
    	int x;
    	int len = strlen(LETTERS);
    
    	printf("\n Please input your ROMAN NUMERALS from the keyboard: ");
    	scanf("%s", LETTERS); /* scanf is very unsafe for reading strings. Consider using fgets. */
    	for (x=0; x<len; ++x)
    	{
    		LETTERS[x] = toupper(LETTERS[x]);
    	}
    }
    
    void choice(void)
    {
    	int c;
    	do
    	{
    		scanf("%i", &c);
    	}		
    	while ((c <=0) || (c >=3));	
    
    	if (c == 1)
    		printf("You have chosen one - file input");
    	else if (c == 2)
    	{
    		printf("You have chosen two - file input");	// FOR KEYBOARD MODE ASK FOR NUMERALS AND COPY INTO "LETTERS"
    		readkeyboard ();
    	}
    }
    
    int main ()
    {			// DECLARE VARIABLES ETC...
    	int answer = 0;
    	int total;
    	int i;
    	int REP;
    	int count [13];
    
    	// CHOOSE BETWEEN KEYBOARD & BATCH MODE
    
    	/* Consider initializing directly in main instead of making small functions for such small code... it doesn't really matter */
    	initialise (LETTERS);
    	initialize (count);
    	/* I don't agree with this - put it in ONE function instead. It's easier to read the code if you ask the question AND take the answers for it in the same function */
    	ask();
    	choice();
    /* ... */
    }
    Some mistakes/corrections you could make. See comments in red.

  7. #22
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    It is very bad to use subtle differences in function names (such as "initialize" and "initialise"). It is almost impossible to see the difference without reading each individual letter. If you want to make two functions different, use prefix or postfix, e.g.

    For example:
    Code:
    InitializeInt();
    InitializeChar();
    or
    Code:
    CharInitialize();
    IntInitialize();
    As Elysia says, using either:
    Code:
    int count[16] = { 0 };
    or
    Code:
    memset(count, 0, sizeof(count));
    would be much better.

    [Don't use ZeroMemory, that is a windows specifc function, and you only want to use such functions when the entire file only contains windows-specific code]

    Not to mention that your code in initialize is broken:
    Code:
    int count [13];
    
    void initialize (int tab[])
    {
    	int i;
    	for (i=0; i<=15; ++i)
    		tab[i] = 0;
    }
    You have 13 elements, but you initialize 16.

    --
    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. #23
    Registered User
    Join Date
    Nov 2007
    Posts
    12

    Cool

    Many thanks for all your help. I have decided to steer clear of functions - don't understand parsing/passing well enough yet...

    What I have done seems to work, although it is more scrappy... The only thing that doesn't work is reading the file in but I don't not whether thats how its being executed or the code.

    Many thanks for all your help. When I get a little better and can help others I surely will...

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    
    int main (void)
    
    {
    
    char tab[16];
    int REP;
    int i;
    int z;
    int total;
    int c;
    int unreal = 0;
    FILE *file_in;
    FILE *file_out;
    
    
    int CountM, CountD, CountC, CountL, CountX, CountV, CountI, CountCM, CountCD, CountXC, CountXL, CountIX, CountIV;
    
    CountM = CountD = CountC = CountL = CountX = CountV = CountI = CountCM = CountCD = CountXC = CountXL = CountIX = CountIV = 0;
    
    
    for (z=0; z<=15; ++z)
    	{
    		tab[z] = 0;
    	}
    
    	printf("\nChoose 1 for FILE input");
    	printf("\nChoose 2 for KEYBOARD input: ");
    
    	do
    	{   
    		scanf("%i", &c);
    	}				
    	while ((c <=0) || (c >=3));	
    
    	if (c == 1)
    	{
    		printf("You have chosen one - file input"); //STILL NEED TO FIGURE OUT CODE TO INPUT A STRING VIA A FILE
    
    		{
     			file_in = fopen("Roman.txt","r");
    			fscanf(file_in, "%s", tab);
    			fclose(file_in);
    			return 0;
    		}
    	}
    
    	else if (c == 2)
    	{
    		printf("You have chosen two - file input");	// (KEYBOARD MODE) ASK FOR NUMERALS AND COPY INTO "LETTERS"
    		
    		printf("\nPlease input your ROMAN NUMERALS from the keyboard: ");
    		scanf("%s", tab);
    	}
    		
    		for (i=0; tab[i] != '\0';i++)
    
    		switch (tab[i]) 
    			{
    				case 'M':
    					REP = 4; 
    					 CountM ++; 
    					if (CountM > REP)
    					{
    						printf("ERROR: You have input too many M's");
    						unreal = 1;
    					}
    					break;
    
    				case 'D':
    					REP = 1; 
    					 CountD ++; 
    					if (CountD > REP)	
    					{
    						printf("ERROR: You have input too many D's");
    						unreal = 1;
    					}					
    					break;
    
    				case 'C':
    					i++;
    					REP = 3; 
    					CountC ++;
    					if (CountC > REP)	
    					{
    						printf("ERROR: You have input too many C's");
    						unreal = 1;
    					}	
    
    					REP = 1;
    					switch (tab[i])
    					{		
    					case '\0':
    						break;
    					case 'M':
    						i++;
    						CountC --; CountCM ++;
    
    						if (CountCM > REP)	
    						{
    							printf("ERROR: You have input too many CM's");
    						unreal = 1;
    						}
    						break;
    					
    					case 'D':
    						i++;
    						CountC --;  CountCD ++;
    						if (CountCD > REP)	
    					{
    						printf("ERROR: You have input too many CD's");
    						unreal = 1;
    					}	
    						break;
    					}i --;
    					break;
    
    				case 'L':
    					
    					REP = 1; 
    					 CountL ++;
    					if (CountL > REP)	
    					{
    						printf("ERROR: You have input too many L's");
    						unreal = 1;
    					}	
    					break;
    
    				case 'X':
    					i++;
    					REP = 3; 
    					CountX ++;
    					if (CountX > REP)	
    					{
    						printf("ERROR: You have input too many X's");
    						unreal = 1;
    					}	
    
    					REP = 1;
    
    					switch (tab[i])
    					{		
    					case '\0':
    						break;
    					case 'C':
    						i++;
    						CountX -- ;  CountXC ++;
    
    						if (CountXC > REP)	
    						{
    							printf("ERROR: You have input too many XC's");
    						unreal = 1;
    						}		
    						break;
    					
    					case 'L':
    						i++;
    						CountX -- ;  CountXL ++;
    						if (CountXL > REP)	
    					{
    						printf("ERROR: You have input too many XL's");
    						unreal = 1;
    					}	
    						break;
    					}i--;
    					break;
    
    				case 'V':
    					REP = 1; 
    					++ CountV;
    					if (CountV > REP)	
    					{
    						printf("ERROR: You have input too many V's");
    						unreal = 1;
    					}	
    					break;
    
    				case 'I':
    					i++;
    					REP = 3; 
    					++ CountI;
    					if (CountI > REP)	
    					{
    						printf("ERROR: You have input too many I's");
    						unreal = 1;
    					}	
    
    					REP = 1;
    
    					switch (tab[i])
    					{		
    					case '\0':
    						break;
    
    					case 'X':
    						i++;
    						CountI -- ; CountIX ++ ;
    
    						if (CountIX > REP)	
    						{
    							printf("ERROR: You have input too many IX's");
    						unreal = 1;
    						}		
    						break;
    					
    					case 'D':
    						i++;
    						CountI --; CountIV ++ ;
    						if (CountIV > REP)	
    					{
    						printf("ERROR: You have input too many IV's");
    						unreal = 1;
    					}	
    						break;
    					}i--;
    					break;
    
    				default:
    					printf("You have input an invalid character"); 
    					
    			}
    	
    	//CALCULATE THE TOTAL OF ALL THE COUNTS MULTIPLIED AGAINST THEIR EQUIVALENT ARABIC VALUE
    
    	total = ((CountM*1000) + (CountD*500) + (CountC*100) + (CountL*50) + (CountX*10)
    		+ (CountV*5) + (CountI) + (CountCM*900) + (CountCD*400) + (CountXC*90) + (CountXL*40) 
    		+ (CountIX*9) + (CountIV*4));
    
    	if (c == 1)
    	{
    		printf("You have chosen one - file input"); //STILL NEED TO FIGURE OUT CODE TO INPUT A STRING VIA A FILE
    
    		{
     			file_out = fopen("new.txt","w");
    			fprintf(file_out, "%i", total);
    			fclose(file_out);
    			return 0;
    		}
    	}
    
    	//PRINT THE ORIGINAL ROMAN NUMERAL AND THE ARABIC CONVERTED VALUE
    
    	printf("\n The value you input in ROMAN NUMERALS was: %s", tab);
    	if (unreal == 0)
    		printf("\n The equivalent value in Arabic numbers is: %i", total);
    	else
    		printf("\n The possible equivalent value of your BADLY CONSTRUCTED ROMAN NUMERAL in Arabic numbers is: %i", total);
    	}

  9. #24
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Steer clear of functions is probably not really the right approach. That's like saying "because using a hammer to fit a piece of glass broke the glass, I won't use a hammer ever again".

    Using the right tool for the job is what it's all about. Functions are great for splitting a task into smaller sections.

    Learning how to split a task into smaller sections is all part of programming.

    Your code as it stands now is a bit too complex, in my opinion. You could certainly make it better by splitting it up.

    In this case, you could also use a table that contains the single or multiple letters [e.g. C, M, IX, CM, etc], the repeat count, and the value for that particlar piece of roman numbers, and then use that table to parse the number, instead of your huge switch statement. Just make sure you check for CM and such before you look for C [simply done by placing those earlier in the table than the "plain" digits].

    --
    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. #25
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Don't use fscaf for reading strings - it's unsafe. Use fgets instead.
    As for reading/writing to files, you're getting there. Remember to check for NULL in your stream returned by fopen. If fopen fails and you call another function, your program will crash trying to read memory from NULL (which will crash on any x86 system).
    When reading, once again, use fgets. When writing, you can use fputs (saves a string to the stream).

  11. #26
    Registered User
    Join Date
    Nov 2007
    Posts
    12

    Lightbulb

    Quote Originally Posted by matsp View Post

    In this case, you could also use a table that contains the single or multiple letters [e.g. C, M, IX, CM, etc], the repeat count, and the value for that particlar piece of roman numbers, and then use that table to parse the number, instead of your huge switch statement.
    Mats
    Could you possibly give me a small example of how I might set up such a table as I haven't yet covered them. I had a look at them but got a bit confused. Maybe if you are unable to give an example you could possibly give me a link to some more information on the subject... with examples

    Many thanks again.

    And thanks Elysia I am looking into fgets...

  12. #27
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Code:
    struct RomanValue
    {
        std::string num;
        int  value;
        int  maxRep;
    };
    
    RomanValue romanValues[] = 
    {
       {"CM", 900, 1 },
       { "M", 1000, 4 },
       { "C", 100, 3 },
       .... 
    };
    --
    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.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Assigning a non null-terminated string
    By BattlePanic in forum C Programming
    Replies: 7
    Last Post: 05-04-2008, 10:02 PM
  2. Replies: 9
    Last Post: 11-23-2007, 12:28 PM
  3. assigning a string to a variable
    By xp5 in forum C Programming
    Replies: 12
    Last Post: 08-30-2007, 01:13 AM
  4. Assigning a String object to char*
    By The Dog in forum C++ Programming
    Replies: 13
    Last Post: 07-14-2002, 04:41 PM
  5. Again Character Count, Word Count and String Search
    By client in forum C Programming
    Replies: 2
    Last Post: 05-09-2002, 11:40 AM