Need some help about if statements and conditionals

**aether** · 07-24-2015

Soo.. heres my problem. My goal in here is to make a program that allows you to play rock paper and scissors. 0 being the rock, 2 being the scissor and 5 being the paper. As long as you use rock (player 1), the program would look like okay but if you try out the other choices, well meh. Anw I'm just a newbie so.. can you please help me out? Thanks

Code:

#include <stdio.h>
#include <conio.h>


int main ()


{
    char p [80], q [80],p1 ,p2 ;
    printf ("Our First Challenger is?: ");
    gets (p);
    printf ("\nOur Second Challenger is?: ");
    gets (q);
    printf ("\nLet the Game of Janken Begin!");
    printf("\n\n%s throws a: ", p);
    scanf("%s", &p1); 
    printf("\n%s throws a: ", q);
    scanf("%s", &p2);
    
    if (p1=='0' || p2=='2')
        {
            printf ("\n %s wins this round!", p);
        }
    else if (p1=='0'|| p2=='5')
        {
            printf ("\n %s wins this round!",q);
        }
    else if (p1=='0' || p2=='0')
        {
            printf ("\n %s,%s Its a tie :)",p,q);
        } 
    else if (p1=='2' || p2=='5')
        {
            printf ("\n %s wins this round!", p);
        }
    else if (p1=='2'|| p2=='0')
        {
            printf ("\n %s wins this round!",q);
        }
    else if (p1=='2' || p2=='2')
        {
            printf ("\n %s,%s Its a tie :)",p,q);    
        } 
    else if (p1=='5' || p2=='0')
        {
            printf ("\n %s wins this round!", p);
        }
    else if (p1=='5'|| p2=='2')
        {
            printf ("\n %s wins this round!",q);
        }
    else if (p1=='5' || p2=='5')
        {
            printf ("\n %s,%s Its a tie :)",p,q);
        }  
    
    return 0;
    getch();
}

**algorism** · 07-24-2015

In your scanf, you're using the %s format to read a character into p1 and p2. p1 and p2 are chars, not strings (i.e., zero-terminated char arrays). Even if you only read a single char into them, the %s format will cause scanf to also add a null character to indicate the end of the string.

My guess as to what's happening is that when you read a string into p2, it writes the character you entered to p2 and then writes the null char over top of p1 (assuming the variables are stored in that order). So p1 always ends up holding '\0' (not to be confused with '0').

You should probably make p1 and p2 ints, since you're reading numbers into them anyway.

And gets() is deprecated since it allows buffer overflows. fgets() is the replacement, but it's a little more complicated to use since it leaves the newline char at the end of the string (if one was read).

Putting your getch() pause AFTER the return statement is a little strange! The best thing to do is to learn to run your programs properly so that you don't need a pause like that. And conio.h is detested by non-windows users (and probably by most windows users), so it's always best to at least remove it (and getch()) before posting.

EDIT:
I just noticed that all your tests are wrong. You should be using &&, not ||.

Also, notice that you obviously don't have to have a separate test for each way p1 and p2 could be equal. Why not just:

Code:

    if (p1 == p2)
        printf ("\n%s, %s Its a tie :)\n", p, q);

You should be printing a newline after the string. Some OS's might not add it for you.

**anduril462** · 07-24-2015

Several things I see:

Stop using conio.h. It's outdated and non-standard. You only use it for getch() which is also non-standard, but you can replace that with the standard getchar() funciton found in stdio.h.
Your getch() statement is after the return in main, so it will never execute.
Don't use gets(), it's prone to buffer overflow since there's no way to limit how many chars it reads into your string (never underestimate a user's ability to do something stupid and break your program). See this link for more info and this link for a better solution using fgets().
You're using scanf with the %s modifier to read in choices to p1 and p2. %s stores a string, i.e. all the characters entered plus the null character to mark the end of the string. Since p1 and p2 are only 1 char each, this means you write to memory you don't own¹. Instead, read a whole line into a buffer (like you do for names) and use the first character as their choice.
You should inform the users of possible choices (0, 2 and 5) and what each one corresponds to. Better yet, use 'r', 'p' and 's' for the inputs. Make sure you allow for lower or upper case input (the tolower() or toupper() function will help here).
Your logic uses || which means if either half of the expression, or both halves are true, then the whole thing is true. Thus, if p1 enters '0', you always take the first if (p1 == '0' || p2 == '2'). The other else if statements with p1 == '0' never trigger. Try && which means both halves must be true for the whole thing to be true.
You should check that the users entered valid input, and print an error message if they didn't. If you want, put this in a loop so your program retries if they give invalid input.
You should use better variable names, and use constants where appropriate (with good names too).

Taking all of that into account gives something like the following

Code:

#define MAX_NAME 80
#define MAX_INPUT 80  // this could be a different value, hence the different name

char player1_name[MAX_NAME], player2_name[MAX_NAME];
char player1_input[MAX_INPUT];
char *p;
...
fgets(player1_name, sizeof(player1_name), stdin);
do {
    printf("%s, enter your choice, R for rock, P for paper or S for scissors: ", player1_name);
    fgets(player1_input, sizeof(player1_input, stdin);
    if input is not r, p or s
        print error
} while input is not r, p or s;
// same for player 2
if p1 is rock and p2 is rock
    tie
else if p1 is rock and p2 is paper
    p2 wins
...
return 0;

¹ Writing to memory you don't own results in undefined behavior, meaning anything, or nothing, can happen. Sometimes your program appears to work, sometimes it may crash.

Guest · 07-24-2015

As another avenue, consider using a result matrix for this game.

Code:

enum Hand {ROCK, PAPER, SCISSORS};
enum Outcome {WIN, DRAW, LOSS};

enum Outcome play(enum Hand player1, enum Hand player2) // returns outcome for player1
{
    // the horizontal rows are player 1, the vertical columns are player 2:
    const enum Outcome matrix[3][3] = {
    //   ROCK, PAPER, SCISSORS
        {DRAW, LOSS,  WIN},  // ROCK
        { WIN, DRAW,  LOSS}, // PAPER
        {LOSS,  WIN,  DRAW}  // SCISSORS
    };
    return matrix[player1][player2];
}

switch(play(SCISSORS, ROCK)) // example hands (Scissors lose to Rock)
{
    case WIN:
        printf("you won!");
        break;
    case DRAW:
        printf("you drew!");
        break;
    case LOSS:
        printf("you lost!");
        break;
}

You may want to use numbers (0, 1, 2) in place of the enums of course, they are just for illustrative purposes so it doesn't look as verbose.

**aether** · 07-26-2015

Thank you guys for the replies ^_^. I'll now try to apply what you've said to me. Thanks for the help

Thread: Need some help about if statements and conditionals

Thread Tools

Search Thread

Display

Need some help about if statements and conditionals

Similar Threads

Constructors and Conditionals

Incrementation in conditionals

conditionals

circle and conditionals

Conditionals