First Program, Please Review!

Hey guys so I made a Rock Paper Scissors program, you enter an input and a computer chooses one and it decides who the winner is.I just wanted some input from you guys where I could improve in this program, I mean I know it compiles and works but I want to be the best programmer as possible and have good practices.

In addition to that, is the random_device the recommended way to generate a random number? I am looking for efficiency here and I hated using rand() :( .

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
  /*RockPaperScissors.cpp by Jacob Amaral*/

#include "stdafx.h"
#include "iostream"
#include "string"
#include "vector"
#include "random"
using std::cout; using std::cin; using std::endl; using std::string; using std::vector; using std::random_device;

string playerChoice, computerChoice;
string choosePlayerOption(), chooseComputerOption();
void calculateWinner(string option1, string option2), startGame();

/*Main method*/
int _tmain(int argc, _TCHAR* argv[])
{
	cout << "Welcome to the RockPaperScissors the game! \n" << endl;
	startGame();
}//end of main
/*startGame method*/
void startGame()
{
	choosePlayerOption();

	if (playerChoice == "rock" || playerChoice == "paper" || playerChoice == "scissors")
	{
		chooseComputerOption();
	}
	calculateWinner(playerChoice, computerChoice);
}//end of startGame
/*playersChoice method - a method that returns the players choice*/
string choosePlayerOption()
{
	do {
		cout << "Please choose either rock paper or scissors :" << endl;
		cin >> playerChoice;
	} 
	while (playerChoice != "rock" && playerChoice != "paper"  && playerChoice != "scissors");

	if (playerChoice == "rock" || playerChoice == "paper" || playerChoice == "scissors")
	{
		return playerChoice;
	}
}//end of playersChoice
/*chooseComputerOption - a method that returns the computers choice*/
string chooseComputerOption()
{
	random_device chooseOption;
	vector<string> options{ "rock", "paper", "scissors" };
	computerChoice = options[chooseOption() % 3];
	cout << "Your opponent chooses : " << computerChoice << "\n" << endl;
	return computerChoice;
}//end of chooseComputerOption
/*calculateWinner - Calculates the winner based on the users input and the computers input*/
void calculateWinner(string option1, string option2)
{
	if (option1 == "rock" && option2 == "rock" || option1 == "paper" && option2 == "paper" ||
		option1 == "scissors" && option2 == "scissors")
	{
		cout << "Its a draw, Please play again" << endl;
		startGame();
	}
	else if (option1 == "rock" && option2 == "scissors" || option1 == "paper" && option2 == "rock" ||
		option1 == "scissors" && option2 == "paper")
	{
		cout << "You win :D !" << endl;
		startGame();
	}
	else{
		cout << "You lose :( !" << endl;
		startGame();
	}
}//end of calculateWinner
Last edited on
int _tmain(int argc, _TCHAR* argv[])
That is not a signature for C++'s main(). The _tmain must be some proprietary MS construct.


Do not use global variable unnecessarily.

Line 25. The choosePlayerOption() should guarantee that.

Line 40. The do-while loop should guarantee that.

Line 55. Return the reult for the caller. Let it show the result.

Lines 61, 67, 71. Recursion that you do not want. Have a do-while loop in startGame() instead.

You could use a lookup-table instead of the if-else in 57-72.
Topic archived. No new replies allowed.