Beginner in C first menu - need critique

This is a discussion on Beginner in C first menu - need critique within the C Programming forums, part of the General Programming Boards category; Hi, I am very new to C programing and am practicing functions, loops, and user input. So I decided a ...

  1. #1
    Registered User jimtuv's Avatar
    Join Date
    May 2010
    Location
    Sylvania, Ohio
    Posts
    94

    Beginner in C first menu - need critique

    Hi, I am very new to C programing and am practicing functions, loops, and user input. So I decided a good way to mix all of that is to make a simple menu. I had a lot of trouble learning how to get user input without a mistake by the user crashing my program. I think I have a good solution my question is this: Is the solution I have come up with a good one? Am I making noob type mistakes here? If so is there any reference material that would help me understand my short comings in this area.

    Here is my menu program.
    Code:
    /*Loop practice Making a menu
    This is a simple menu that allows
    three choices of destination and
    quits when the user hits q
    */
    
    #include <stdio.h>
    
    /* Function templates */
    void functone (void);
    void functtwo (void);
    void functthree (void);
    
    
    int main ()
    {
    	// set up conditions for the loop
    	
    	int choice;
    	char ch;
    	choice = 0;
    	
    	// I used a do while loop to insure that it would run once
    	do{
    		// print menu
    		printf ( "Make your choice.\n");
    		printf ( "1 - 1st  program\n" );
    		printf ( "2 - 2nd program\n" );
    		printf ( "3 - 3rd  program\n" );
    		printf ( "q to quit\n");
    		
    		// get user input and clear out stdin
    		choice = getchar ();
    		
                    // this part clears the stdin of leftovers
    		ch = getchar ();
    		while ( ch != '\n' ) ch = getchar ();
    		
    		// Call the desired function
    		if (choice == 49 ) {
    			functone ();
    		}
    		if (choice == 50 ) {
    			functtwo ();
    		}
    		if (choice == 51 ) {
    			functthree ();
    		}
    	}
    	// Check for escape code
    	while ( choice != 113);
    	
    	// just to verify I finished
    	printf ( "Done\n");
    		
    		return 0;
    		
    }
    
    // The first called function on the menu
    void functone (void)
    {
    	printf ( "You chose one\n");
    }
    
    // The second called function on the menu
    void functtwo (void)
    {
    	printf ( "You chose two\n");
    }
    
    // The third called function on the menu
    void functthree (void)
    {
    	printf ( "You chose three\n");
    }

  2. #2
    Registered User NeonBlack's Avatar
    Join Date
    Nov 2007
    Posts
    435
    You should use character literals instead of magic numbers. It makes your code much more readable, for example '1' instead of 49, 'q' instead of 133.

    Since only one of the if statements can be taken, you should use if... else if... or even better, switch:
    Code:
    switch (choice)
    {
        case '1': functone(); break;
        case '2': functtwo(); break;
        /* ... */
        default: /* Invalid choice! */
    }
    Some very minor points:
    I believe in C, main()'s header should be int main(void) or int main(int, char**).
    Code:
    ch = getchar ();
    		while ( ch != '\n' ) ch = getchar ();
    this is fine, but can be written terser and without the need for another variable as
    Code:
    while(getchar()!='\n');
    Last edited by NeonBlack; 05-16-2010 at 11:06 PM.
    I copied it from the last program in which I passed a parameter, which would have been pre-1989 I guess. - esbo

  3. #3
    Registered User jimtuv's Avatar
    Join Date
    May 2010
    Location
    Sylvania, Ohio
    Posts
    94
    Thank you for the advice I have modified the program with your suggestions. I also made a new template for my IDE it was defaulting with the int main (). So I redid it to int main (void).

    Here is the revised program.

    Code:
    /*Loop practice Making a menu
    This is a simple menu that allows
    three choices of destination and
    quits when the user hits q
    */
    
    #include <stdio.h>
    
    /* Function templates */
    void functone (void);
    void functtwo (void);
    void functthree (void);
    
    
    int main (void)
    {
    	// set up conditions for the loop
    	
    	int choice;
    	choice = 0;
    	
    	// I used a do while loop to insure that it would run once
    	do{
    		// print menu
    		printf ( "Make your choice.\n");
    		printf ( "1 - 1st  program\n" );
    		printf ( "2 - 2nd program\n" );
    		printf ( "3 - 3rd  program\n" );
    		printf ( "q to quit\n");
    		
    		// get user input and clear out stdin
    		choice = getchar ();
    		// this part clears the stdin of leftovers
    		while ( getchar() != '\n' ) ;
    		
    		// Call the desired function
    		if (choice == '1' ) {
    			functone ();
    		}
    		if (choice == '2' ) {
    			functtwo ();
    		}
    		if (choice == '3' ) {
    			functthree ();
    		}
    	}
    	// Check for escape code
    	while ( choice != 'q');
    	
    	// just to verify I finished
    	printf ( "Done\n");
    		
    		return 0;
    		
    }
    
    // The first called function on the menu
    void functone (void)
    {
    	printf ( "You chose one\n");
    }
    
    // The second called function on the menu
    void functtwo (void)
    {
    	printf ( "You chose two\n");
    }
    
    // The third called function on the menu
    void functthree (void)
    {
    	printf ( "You chose three\n");

  4. #4
    Registered User NeonBlack's Avatar
    Join Date
    Nov 2007
    Posts
    435
    Looks better, but you still didn't do the else thing. This is what I mean.
    Code:
    // Call the desired function
    if (choice == '1' ) {
    	functone ();
    }
    else if (choice == '2' ) {
    	functtwo ();
    }
    else if (choice == '3' ) {
    	functthree ();
    }
    else {
     /* code for invalid choice */
    }
    You want this because once an if evaluates true, you don't care about any other code path.
    I copied it from the last program in which I passed a parameter, which would have been pre-1989 I guess. - esbo

  5. #5
    Registered User jimtuv's Avatar
    Join Date
    May 2010
    Location
    Sylvania, Ohio
    Posts
    94
    I'm sorry I completely missed that block of code with switch in it. Didn't even notice it was there. I haven't seen switch yet though I am sure it will be brought up soon in my reading. I will do a quick search for it and see what it's all about. It sure looks easy to implement though the code is much cleaner then all the if statements.

    From what I read so far switch works only with constant integral expressions like I have in this menu. If the choices were letters I would not be able to use switch in this manner correct me if I am wrong. Also the break command keeps from falling through and executing the next command. I will have to remember this. Here is the revised code with switch included.

    One problem I just noticed is that this will only work for number up to 9. I will have to think about how to add the second digit if I were to go with numbers higher then 9

    Code:
    /*Loop practice Making a menu
    This is a simple menu that allows
    three choices of destination and
    quits when the user hits q
    */
    
    #include <stdio.h>
    
    /* Function templates */
    void functone (void);
    void functtwo (void);
    void functthree (void);
    
    
    int main (void)
    {
    	// set up conditions for the loop
    	
    	int choice;
    	choice = 0;
    	
    	// I used a do while loop to insure that it would run once
    	do{
    		// print menu
    		printf ( "Make your choice.\n");
    		printf ( "1 - 1st  program\n" );
    		printf ( "2 - 2nd program\n" );
    		printf ( "3 - 3rd  program\n" );
    		printf ( "q to quit\n");
    		
    		// get user input and clear out stdin
    		choice = getchar ();
    		// this part clears the stdin of leftovers
    		while ( getchar() != '\n' ) ;
    		
    		// Call the desired function
    		switch ( choice ){
    		case '1' : functone (); break;
    		case '2' : functtwo (); break;
    		case '3' : functthree (); break;
    		}
    	}
    	// Check for escape code
    	while ( choice != 'q');
    	
    	// just to verify I finished
    	printf ( "Done\n");
    		
    		return 0;
    		
    }
    
    // The first called function on the menu
    void functone (void)
    {
    	printf ( "You chose one\n");
    }
    
    // The second called function on the menu
    void functtwo (void)
    {
    	printf ( "You chose two\n");
    }
    
    // The third called function on the menu
    void functthree (void)
    {
    	printf ( "You chose three\n");
    }
    Last edited by jimtuv; 05-17-2010 at 12:26 AM.

  6. #6
    VIM addict
    Join Date
    May 2009
    Location
    Chennai, India
    Posts
    43
    jimtuv, one thing is that you don't need to use the single quote( ' ) in your case statement, as you are dealing with integers. When you are using characters then you need to use the single quotes. For example

    In case of using Integer
    Code:
    int choice;
    ...
    ...
    switch (choice)
    {
        case 1: printf("I am 1\n"); break;
        case 10: printf("I am 10\n"); break;
        default: printf("I am other than 1 and 10\n"); break;
    }
    In case of using Character
    Code:
    char choice;
    ...
    ...
    switch (choice)
    {
        case 'a': printf("I am A\n"); break;
        default: printf("I am other than A\n"); break;
    }

  7. #7
    Registered User jimtuv's Avatar
    Join Date
    May 2010
    Location
    Sylvania, Ohio
    Posts
    94
    Actually I think that I do need to use the single quotes in this situation. If you look at the code the variable choice gets the ASCII characters for 1, 2, 3 and q being 49, 50, 51 and 113 from the getchar (); fuction. As NeonBlack reminded me It makes it more readable having the character they represent rather then a magic number that just appears out of nowhere. I may be wrong I am pretty new at this.
    Last edited by jimtuv; 05-18-2010 at 09:11 AM.

  8. #8
    VIM addict
    Join Date
    May 2009
    Location
    Chennai, India
    Posts
    43
    jimtuv, what you had is correct only i didn't noticed that part in your code.

  9. #9
    Registered User jimtuv's Avatar
    Join Date
    May 2010
    Location
    Sylvania, Ohio
    Posts
    94
    No problem cablu I am sure grateful you took the time to look at my code. Every input is helpful.

    I decided to understand how the menu works that it would be good to try it out in a simple guessing game. In this game the computer picks a random number in a given range and allow a certain number of guesses for the user to find the answer.

    Here the menu allows the user to input the desired difficulty and then run the game function. I know the input function is not right yet but I am not far enough along in my studies to make bullet proof input yet. I want to understand each function before going to much further.

    I purposely broke it down in as many functions as I could to get practice in setting up and calling functions.

    I have compile and run it and it seems to work as I would expect. Here is the Menu in the game.

    It's amazing how small the main function is.

    Code:
    /* A guessing game from James Tuvell
    
    In this game the computer picks a random number in
    a certain range and the user will have a specified number
    of tries to find that number
    
    */
    
    #include <stdio.h>
    #include <time.h>
    
    int Menu (void);
    void Instructions (int lownumb, int highnumb, int guesses );
    int MenuInput (void);
    int GameInput (void);
    int Randnumb ( int lownumb, int highnumb );
    void Game ( int lownumb, int highnumb, int guesses );
    
    int main (void){
    	
    	// declare and initialize variables
    	int answer;
    		
    	// main program loop
    	do {
    		// jump to menu
    		
    		answer = Menu ();
    				
    		/* On the menu selection jump to the Game function
    			with the apropriate settings for that game
    			difficulty.
    		*/
    		switch (answer){
    			case '1' : Game ( 1, 20, 5); break;
    			case '2' : Game ( 1, 50, 10); break;
    			case '3' : Game ( 1, 99, 20); break;
    		}
    			
    		
    	}
    	while ( answer != 'q' );
    	
    	return 0;
    }
    
    /* This is the menu function it only prints
    *   the menu. It returns no value 
    */
    int Menu (void){
    	// menu function
    	int answer;
    	int numb;
    	
    	printf ( "\n\n" );
    	printf ( "Chose you level of dificulty \n");
    	printf ( "1 - Easy between 1 and 20 with 5 guesses\n");
    	printf ( "2 - Medium between 1 and 50 with 10 guesses\n");
    	printf ( "3 - Hard between 1 to 99 with 20 guesses\n");
    	printf ( "q -to quit\n\n");
    	
    	answer = MenuInput ();
    	return answer;
    	
    }
    
    /* This prints the instructions for the game 
    	based on the choice made from the menu
    */
    void Instructions ( int lownumb, int highnumb, int guesses ){
    	// print instruction for game
    	
    	printf ( "\n" );
    	printf ( "I will think of a number " );
    	printf ( "between %d and %d\n", lownumb, highnumb );
    	printf ( "you have %d guesses to find it.\n\n", guesses );
    	
    
    }
    
    /* This gets user input and returns the answer
    *  it also will clear out the stdin buffer before
    *  returning
    */
    int MenuInput (void){
    	
    	int answer;
    		
    	answer = getchar ();
    		
    	while ( getchar () != '\n' );
    	
    	
    	return answer;
    }
    
    /* This is the input for the game
    	I still need to check for errors
    	Have to add something to keep 
    	letters from dumping my buffer
    */
    
    int GameInput (void){
    	
    	int answer;
    	
    	scanf ( "%d", &answer );
    	while ( getchar () != '\n' );
    	
    	return answer;
    	
    }
    
    /* This is the random number generator that gets
    	a range from the user and generates a random
    	number in that range. It uses the time to initialize
    	to prevent duplicate numbers
    */
    
    int Randnumb ( int lownumb, int highnumb ){
    	
    	int randnumb;
    	
    	// initialize random number generator
    	srand ( time (NULL) );
    	
    	randnumb = ( rand () % ( highnumb-lownumb ) + lownumb) ;
    	
    	return randnumb;
    }
    
    /* This function is the game function. It takes in the range
    	of the number to be guessed and the number of  guesses
    	that the user is alowed.
    */
    
    void Game ( int lownumb, int highnumb, int guesses ){
    	
    	// declare variables
    	int numb_to_guess;
    	int guess;
    	int digit;
    	int i;
    	
    	// print instructions 
    	Instructions ( lownumb , highnumb, guesses );
    	
    	// chose a number for user to guess
    	numb_to_guess = Randnumb ( lownumb, highnumb );
    	
    	// Set the number of digits for user input
    	digit = 2;
    	
    	// Main game loop
    	for ( i = 0 ; i < guesses ; i++) {
    		
    		printf ( "Enter your guess:");
    		guess = GameInput ( );
    		
    		if ( guess > numb_to_guess ){
    			printf ( "Your too high try again!\n" );
    		}
    		if ( guess < numb_to_guess ) {
    			printf ( "Your too low try again!\n");
    		}
    		if (guess == numb_to_guess ) {
    			printf ( "You Win!!!\n");
    			return;
    		}
    		
    	}
    	
    	printf ("You loose!!! it was %d\n",numb_to_guess);
    		
    }

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Checking array for string
    By Ayreon in forum C Programming
    Replies: 87
    Last Post: 03-09-2009, 04:25 PM
  2. Replies: 2
    Last Post: 03-05-2009, 10:25 AM
  3. Another syntax error
    By caldeira in forum C Programming
    Replies: 31
    Last Post: 09-05-2008, 02:01 AM
  4. Constructive Feed Back (Java Program)
    By xddxogm3 in forum Tech Board
    Replies: 12
    Last Post: 10-10-2004, 04:41 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21