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");
}

---

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');

---

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");

---

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'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");
}

---

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;
}

---

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.

jimtuv, what you had is correct only i didn't noticed that part in your code. :)

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);
               
}

---