patterncppModerate
Simple Peg game
Viewed 0 times
gamesimplepeg
Problem
I'm new to coding and this code pretty much goes to the extent of my current knowledge I'm looking for ways to improve this code. This code compiles and runs fine in Visual Studio 2013. I'm also using Windows 8.1 as my current OS for the computer I'm writing and compiling on.
I'd like a review over anything and any errors with the math involved in the game or the errors with the game.
What this code is suppose to do and from my testing does. Is be a simple game in which there are two players taking pegs off a ring one player is the user (human), the other player is a computer. Depending on the what moves the user makes is the computer responds with an ideal move. If there is no ideal move then the computer makes a random move of taking 1 ring off a random peg basically attempting to slow the game down giving more chances for the human user to make a mistake.
The math involved in this basically makes it so player 1 can always win. The computer is to know every ideal move and react based on human user moves. But since the computer can go second he is then just waiting for a error made by the human user or trying to slow down the game.
```
#include
#include
#include
#include
using namespace std;
int main()
{
char cAgain;
do
{
//Variable Declartion
int x, PegTotal, PegOne = 3, PegTwo = 5, PegThree = 7, Player, y;
bool switchRepeat = true;
cout > Player;
if (Player == 1)
{
cout > x;
if ((x == 1) && (PegOne == 0) || (x == 2) && (PegTwo == 0) || (x == 3) && (PegThree == 0))
{
cout > y;
if (y > PegOne)
{
cout > y;
if (y > PegTwo)
{
cout > y;
if (y > PegThree)
{
I'd like a review over anything and any errors with the math involved in the game or the errors with the game.
What this code is suppose to do and from my testing does. Is be a simple game in which there are two players taking pegs off a ring one player is the user (human), the other player is a computer. Depending on the what moves the user makes is the computer responds with an ideal move. If there is no ideal move then the computer makes a random move of taking 1 ring off a random peg basically attempting to slow the game down giving more chances for the human user to make a mistake.
The math involved in this basically makes it so player 1 can always win. The computer is to know every ideal move and react based on human user moves. But since the computer can go second he is then just waiting for a error made by the human user or trying to slow down the game.
```
#include
#include
#include
#include
using namespace std;
int main()
{
char cAgain;
do
{
//Variable Declartion
int x, PegTotal, PegOne = 3, PegTwo = 5, PegThree = 7, Player, y;
bool switchRepeat = true;
cout > Player;
if (Player == 1)
{
cout > x;
if ((x == 1) && (PegOne == 0) || (x == 2) && (PegTwo == 0) || (x == 3) && (PegThree == 0))
{
cout > y;
if (y > PegOne)
{
cout > y;
if (y > PegTwo)
{
cout > y;
if (y > PegThree)
{
Solution
using namespace std;This can be a bad habit to develop. See Why is using namespace std bad practice?
//Variable DeclartionTypo.
//Variable Declarations
int x, PegTotal, PegOne = 3, PegTwo = 5, PegThree = 7, Player, y;
bool switchRepeat = true;In C++, you don't have to declare your variables at the start. You can declare them just before you start using them. This helps with keeping track of what each variable is.
In this case, you probably should declare some of those in a class. However, that's a pretty big leap from the current state of the code.
Try to capitalize consistently. The general standards for a variable name are either camelCase or snake_case. The
switchRepeat variable is camelCase, so you should probably make Player and PegTotal into player and pegTotal respectively. If you see yourself giving variables names with numbers in them, consider using a collection to hold them instead.
std::array pegs = { 3, 5, 7 };I make collections plural. Now
pegs[0] is equal to 3, just as PegOne was. system("PAUSE");
system("CLS");Using system commands like this can be risky. See Edward's answer here for more information.
You have an incredible amount of nesting and should use functions to abstract out some of the functionality. E.g.
void showHelp() {
cout << "Welcome to the 3 Peg game!!!" << endl;
cout << "This is a 2 player game." << endl << endl;
cout << "The game has 3 pegs and 15 rings." << endl
<< "The first peg has 3 rings, the second peg has 5, and the third peg has 7." << endl
<< "The object of the game is to get the other player to take off the last ring." << endl << endl;
system("PAUSE");
system("CLS");
cout << "The rules of the game are as follow:" << endl
<< "1. You must take at least one ring off a peg per turn." << endl
<< "2. You can only take rings off a single peg in a given turn." << endl
<< "3. You can take as many rings off a single peg as you like." << endl
<< "4. You cannot skip a turn." << endl
<< "5. The player who takes the last ring off loses." << endl << endl;
system("PAUSE");
system("CLS");
}Now you can just call
showHelp() whenever you want to display this. cout > Player;
if (Player == 1)
{
cout << "You are player one!" << endl
<< "You will go first." << endl << endl;
goto check;
}
else if (Player == 2)
{
cout << "You are player 2!" << endl
<< "You will go second" << endl << endl;
cout << "Game Master takes 3 rings off peg 3" << endl;
PegThree = 4;
do
{
check:In this case, it's trivial to write the code such that a
goto isn't necessary. Note that you don't have to nest the do with check: in it inside the else clause. do
{
cout > player;
} while ( player != 1 && player != 2 );
if (player == 1)
{
cout << "You are player one!" << endl
<< "You will go first." << endl << endl;
}
else
{
cout << "You are player 2!" << endl
<< "You will go second" << endl << endl;
cout << "Game Master takes 3 rings off peg 3" << endl;
pegs[2] = 4;
}
do
{
check:This way, you get rid of two layers of nesting as well as eliminating a
goto. But we probably want to go ahead and make a couple functions out of this too. enum Turn { YOU, GAME_MASTER };But first, let's declare an
enum. With this, we can specify who is doing what without using integers. void removeRings(std::array & pegs, std::size_t peg, int quantity, Turn turn)
{
switch ( turn )
{
case YOU:
std::cout << "You take ";
break;
case GAME_MASTER:
std::cout << "Game Master takes ";
break;
}
std::cout << quantity << " rings off peg "
std::cout << peg + 1 << endl;
pegs[peg] -= quantity;
}This function just provides a standard way to represent a move. By defining a function, we put all instances where we're announcing a move in the same place. In your original code, if you wanted to change the way that this was output, you'd have to make changes all over the place.
We introduce the idea of function parameters. The first parameter,
pegs, is passed by reference. You can tell because it has a & between the type and the variable name that will be used in the function. We give pegs the same name in the function as it had in the caller, but we did not have to do so. It was simply the best name in both places. Passed by reference means that we don't have to make a new copy of the object and allows us to modify the original object while in the function. The other three parameters are passed by value. Code Snippets
using namespace std;//Variable Declartion//Variable Declarations
int x, PegTotal, PegOne = 3, PegTwo = 5, PegThree = 7, Player, y;
bool switchRepeat = true;std::array<int, 3> pegs = { 3, 5, 7 };system("PAUSE");
system("CLS");Context
StackExchange Code Review Q#74623, answer score: 18
Revisions (0)
No revisions yet.