rock, paper, scissors game

krnacandrej (12)
Hi I made rps game and I want to know if there is a way how to improve this code. I think in who_win function is too many ifs and i dont know how it makes better. Any ideas? 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
 
#include <iostream>
using namespace std;

char who_win ( char cp, char user );

int main()
{
    string user_string, cp_string;
    int random_number, user_goal = 0, cp_goal = 0;
    char result, user, cp, option;
    bool koniec = false;
    srand ( time (NULL) );
    
    
    
    
while(!koniec)
{
              random_number = rand()%3+1;
              if( random_number == 1 ) { cp = 'r'; cp_string = "rock"; }
              if( random_number == 2 ) { cp = 'p'; cp_string = "paper"; }
              if( random_number == 3 ) { cp = 's'; cp_string = "scissors"; }
              
              cout << "\nLET'S START.... [r]rock  [p]paper  [s]scissors\n";
              cin >> user;
              
              cout << "\n\tcp: " << cp_string << endl;     
              result = who_win(cp,user);
                           
              if( result == 'x' )
              {
                  cout << "\tERROR!" << endl;
                  cout << "\n[e]end game   [a]again" << endl;
                  cin >> option;
                  }
                  
              if( result == 'w' )
              {
                  cout << "\tYOU WIN!" << endl;
                  user_goal++;
                  cout << "\n[e]end game   [a]again" << endl;
                  cin >> option;
                  }
              
              if( result == 'l' )
              {
                  cout << "\tYOU LOSE!" << endl;
                  cp_goal++;
                  cout << "\n[e]end game   [a]again" << endl;
                  cin >> option;
                  }
              
              if( result == 't' )
              {
                  cout << "\tIT'S TIE!" << endl;
                  cout << "\n[e]end game   [a]again" << endl;
                  cin >> option;
                  }
                  if( option == 'e' ) koniec = true;              
}

cout << "\ncp score: " << cp_goal << endl;
cout << "your score: " << user_goal << endl;

if ( cp_goal < user_goal ) cout << "!____YOU ARE WINNER____!";
if ( cp_goal > user_goal ) cout << "!____COMPUTER IS WINNER____!";
if ( cp_goal == user_goal ) cout << "!____IT'S TIE____!";

cout <<"\n\npress enter to end program....";
cin.get();
cin.get();
    return 0;
}

char who_win ( char cp, char user)
{
     char result;
     
 
     if( cp == user ) result = 't';         
     if( cp=='p' && user=='r' ) result = 'l';      
     if( cp=='r' && user=='p' ) result = 'w';
     if( cp=='p' && user=='s' ) result = 'w';
     if( cp=='s' && user=='p' ) result = 'l';
     if( cp=='r' && user=='s' ) result = 'l';
     if( cp=='s' && user=='r' ) result = 'w';
     if( user != 'p' && user != 's' && user != 'r' ) result = 'x'; 
     return result;
}
Last edited on
Nisheeth (545)
Firstly, you should spread out the code a bit. Like on line 20 and 21, thay would be much more readable if they took a bit more lines.

As for reducing the number of ifs in who_win

1
2
3
4
5
 
if (
     (cp == 'r' && user == 's') ||
     (cp == 'p' && user == 'r') ||
     (cp == 's' && user == 'p')
   )


you can format the code in this way to reduce the number of ifs to three!
krnacandrej (12)
thats good idea thank you
andywestken (4094)
You should consider using else to cut down on the number of comparisons which are made.

And eliminate the duplicate code!

Andy
krnacandrej (12)
I remade ifs and added function end_game (to eliminate duplicate code) is it better now ? 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
 
#include <iostream>
#include <time.h>
#include <string>
using namespace std;

char who_win ( char cp, char user );
char end_program();

int main()
{
    string user_string, cp_string;
    int random_number, user_goal = 0, cp_goal = 0;
    char result, user, cp, option;
    bool koniec = false;
    srand ( time (NULL) );
    

while(!koniec)
{
              random_number = rand()%3+1;
              if( random_number == 1 ) 
                 { cp = 'r'; 
                    cp_string = "rock"; }
	      else if( random_number == 2 )
                 { cp = 'p';  
                    cp_string = "paper"; }
	      else if( random_number == 3 )
                 { cp = 's';
                    cp_string = "scissors"; }
              
              cout << "\nLET'S START.... [r]rock  [p]paper  [s]scissors\n";
              cin >> user;
              
              cout << "\n\tcp: " << cp_string << endl;     
              result = who_win(cp,user);
                           
              
              if( result == 'w' )
              {
                  cout << "\tYOU WIN!" << endl;
                  user_goal++;
                  option=end_program();
                  }
              
              else if( result == 'l' )
              {
                  cout << "\tYOU LOSE!" << endl;
                  cp_goal++;
                  option=end_program();
                  }
              
              else if( result == 't' )
              {
                  cout << "\tIT'S TIE!" << endl;
                  option=end_program();
                  }
			  else
			  {
                  cout << "\tERROR!" << endl;
                  option=end_program();
                  }
                  
                  if( option == 'e' ) koniec = true;              
}

cout << "\ncp score: " << cp_goal << endl;
cout << "your score: " << user_goal << endl;

if ( cp_goal < user_goal ) cout << "!____YOU ARE WINNER____!";
else if ( cp_goal > user_goal ) cout << "!____COMPUTER IS WINNER____!";
else cout << "!____IT'S TIE____!";

cout <<"\n\npress enter to end program....";
cin.get();
cin.get();
    return 0;
}

char who_win ( char cp, char user)
{
     char result;
     
 
     if ( cp == user ) result = 't';         
     
     else if ((cp == 'r' && user == 's') || (cp == 'p' && user == 'r') || (cp == 's' && user == 'p'))
             { result = 'l'; }
     
     else if ((cp == 'r' && user == 'p') || (cp == 'p' && user == 's') || (cp == 's' && user == 'r'))
             { result = 'w'; }
     
     else result = 'x'; 
    
     return result;
}       

char end_program()
{
	char choose;
	cout << "\n[e]end game   [a]again" << endl;
    cin >> choose;
	return choose;
}


Last edited on
andywestken (4094)
Yes it's better than before!

But you still have unnecessary duplicate code. I can see end_program being called in all the cases of you if-else tests. As it's called whatever the condition, it doesn't need to be handled inside the if-else.

Andy

P.S. One other comment. The variable declarations are very C. In C++you generally declare the variables as close as possible to the point of first use. And it's good form to initialize all variables (this makes it easier to spot problems when debugging).
Last edited on
krnacandrej (12)
of course it isnt necessary put end_game function to ifs....I see it now.
I remade declarations of variables but should I initialize char and string variables? For example should I do it like this: char result = 'x'; char option = 'x'; string user_string = "x"; ?
thx a lot for your advices...

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
 
#include <iostream>
#include <time.h>
#include <string>
using namespace std;

char who_win ( char cp, char user );
char end_program();

int main()
{
    int user_goal = 0, cp_goal = 0;
	bool koniec = false;
    srand ( time (NULL) );
    

while(!koniec)
{
              string user_string, cp_string;
			  char user, cp, result;
			  int random_number = 0;
			  random_number = rand()%3+1;
              if( random_number == 1 ) { cp = 'r'; 
                                         cp_string = "rock"; }
			  else if( random_number == 2 ) { cp = 'p';  
                                         cp_string = "paper"; }
			  else if( random_number == 3 ) { cp = 's';
                                         cp_string = "scissors"; }
              
              cout << "\nLET'S START.... [r]rock  [p]paper  [s]scissors\n";
              cin >> user;
              
              cout << "\n\tcp: " << cp_string << endl;     
              result = who_win(cp,user);
                           
              if( result == 'w' )
              {
                  cout << "\tYOU WIN!" << endl;
                  user_goal++;
                  }
              
              else if( result == 'l' )
              {
                  cout << "\tYOU LOSE!" << endl;
                  cp_goal++;
                  }
              
              else if( result == 't' )
              {
                  cout << "\tIT'S TIE!" << endl;
                  }
			  else
			  {
                  cout << "\tERROR!" << endl;
                  }
                  char option;
				  option=end_program();
                  if( option == 'e' ) koniec = true;              
}

cout << "\ncp score: " << cp_goal << endl;
cout << "your score: " << user_goal << endl;

if ( cp_goal < user_goal ) cout << "!____YOU ARE WINNER____!";
else if ( cp_goal > user_goal ) cout << "!____COMPUTER IS WINNER____!";
else cout << "!____IT'S TIE____!";

cout <<"\n\npress enter to end program....";
cin.get();
cin.get();
    return 0;
}

char who_win ( char cp, char user)
{
     char result;
     
 
     if ( cp == user ) result = 't';         
     
     else if ((cp == 'r' && user == 's') || (cp == 'p' && user == 'r') || (cp == 's' && user == 'p'))
             { result = 'l'; }
     
     else if ((cp == 'r' && user == 'p') || (cp == 'p' && user == 's') || (cp == 's' && user == 'r'))
             { result = 'w'; }
     
     else result = 'x'; 
    
     return result;
}       

char end_program()
{
	char choose;
	cout << "\n[e]end game   [a]again" << endl;
    cin >> choose;
	return choose;
}


andywestken (4094)
You don't have to initialize string variables unless you want to set them to a particular value to start with (as opposed to just blank) as string is a class with a constructor to handle its initialization.

I do initialize all my intrinsic (non-class) variables. But most of them are declared at the point of first use, so the amount of explicit initialization is quite small.

I also follow the one variable declaration per line convention, but then I'm pedantic.

Initialization does allow you to tidy up end_program a little bit:

1
2
3
4
5
6
7
8
9
10
11
12
13
 
char who_win ( char cp, char user)
{
    char result = 'x';
 
    if ( cp == user ) 
        result = 't';
    else if ((cp == 'r' && user == 's') || (cp == 'p' && user == 'r') || (cp == 's' && user == 'p'))
        result = 'l';
    else if ((cp == 'r' && user == 'p') || (cp == 'p' && user == 's') || (cp == 's' && user == 'r'))
        result = 'w';
    
     return result;
}
Last edited on
closed account (DSLq5Di1)
An example of what you could do:-
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
 
#include <windows.h>
#include <iostream>
#include <string>
#include <ctime>
#include <cmath>

using namespace std;

enum total { draw, win, loss };
const string str[6] = { "Rock", "Scissors", "Paper", "\tIT'S TIE!", "\tYOU WIN!", "\tYOU LOSE!" };

int main()
{
    srand(time(0));

    cout << "\nLET'S START.... [r]rock  [p]paper  [s]scissors\n\n";
    int scores[3] = {};

    for (char input, exit = false; !exit; Sleep(10))
    {
        cout << "> ";
        cin >> input;
        int result, comp = rand() % 3;

        switch (input)
        {
            case 'r': case 'p':	case 's':
            {
                int user = abs(input - 'r');
                result = (2 * user + comp) % 3;
                scores[result]++;

                cout << '\t' << str[user] << " vs " << str[comp] << '\n' << str[result+3] << endl;
                break;
            }
            case 'e':
                exit = true;
                break;
            default:
                cout << "\tERROR!" << endl;
        }
        if (!exit)
            cout << "\n------------------------------------------"
                 << "\n[e]nd game  [r]rock  [p]paper  [s]scissors\n" << endl;
    }

    if (scores[win] == scores[loss])
        cout << "\n\t!____IT'S TIE____!";
    else if (scores[win] > scores[loss])
        cout << "\n\t!____YOU ARE WINNER____!";
    else
        cout << "\n\t!____COMPUTER IS WINNER____!";
	
    cout << "\n\tWin: " << scores[win] << ", Draw: " << scores[draw] << ", Loss: " << scores[loss];
    cout << "\n\nPress enter to end program...." << endl;

    cin.sync();
    cin.ignore();

    return 0;
}
Last edited on
krnacandrej (12)
Sloppy9 can you explain me this part ? : 

1
2
3
 
int user = abs(input - 'r');
                result = (2 * user + comp) % 3;
                scores[result]++;

Topic archived. No new replies allowed.