patterncppMinor
TicTacToe game with large switch statements
Viewed 0 times
withtictactoestatementsgamelargeswitch
Problem
This 5 classes:
Is there a better way to update the board besides a large switch case? I have yet to think of a way to hold the two array indices of the board. Please give me some constructive criticism on my console TicTacToe game? It would be difficult to post all 10 files though, so I've uploaded it to GitHub (but only review the code in this post).
```
void CBoard::iCustomUpdateBoard(CPlayer obj, CInput UserInput)
{
unsigned keyInput = UserInput->getSingleNumPadKey();
//std::cout Graphic.iClearScreen();
obj->Graphic.iPrintBoard(this);
std::cout playerMove(this);
}
else
board[2][0] = obj->getUserID();
moves++;
//std::cout board[2][0] Graphic.iClearScreen();
obj->Graphic.iPrintBoard(this);
std::cout playerMove(this);
}
else
board[2][1] = obj->getUserID();
moves++;
break;
case (3):
if (board[2][2] != '*')
{
std::cout Graphic.iClearScreen();
obj->Graphic.iPrintBoard(this);
std::cout playerMove(this);
}
else
board[2][2] = obj->getUserID();
moves++;
break;
case (4):
if (board[1][0] != '*')
{
std::cout Graphic.iClearScreen();
obj->Graphic.iPrintBoard(this);
std::cout playerMove(this);
}
else
board[1][0] = obj->getUserID();
moves++;
break;
case (5):
if (board[1][1] != '*')
{
std::cout Graphic.iClearScreen();
CBoard, CInput, CPlayer, Console, Graphics, and the main file, Test, which is actually main.cpp (but I named it poorly). Specifically, I use a switch to update the board, which is a 2D char array.Is there a better way to update the board besides a large switch case? I have yet to think of a way to hold the two array indices of the board. Please give me some constructive criticism on my console TicTacToe game? It would be difficult to post all 10 files though, so I've uploaded it to GitHub (but only review the code in this post).
```
void CBoard::iCustomUpdateBoard(CPlayer obj, CInput UserInput)
{
unsigned keyInput = UserInput->getSingleNumPadKey();
//std::cout Graphic.iClearScreen();
obj->Graphic.iPrintBoard(this);
std::cout playerMove(this);
}
else
board[2][0] = obj->getUserID();
moves++;
//std::cout board[2][0] Graphic.iClearScreen();
obj->Graphic.iPrintBoard(this);
std::cout playerMove(this);
}
else
board[2][1] = obj->getUserID();
moves++;
break;
case (3):
if (board[2][2] != '*')
{
std::cout Graphic.iClearScreen();
obj->Graphic.iPrintBoard(this);
std::cout playerMove(this);
}
else
board[2][2] = obj->getUserID();
moves++;
break;
case (4):
if (board[1][0] != '*')
{
std::cout Graphic.iClearScreen();
obj->Graphic.iPrintBoard(this);
std::cout playerMove(this);
}
else
board[1][0] = obj->getUserID();
moves++;
break;
case (5):
if (board[1][1] != '*')
{
std::cout Graphic.iClearScreen();
Solution
The code you've posted is highly repetitive. You have 9 repetitions of nearly identical code, with a couple of instances of
I'd rather see (for just one possibility) a small array defining the mapping from input number (1..9) to 2D board position:
Using that, the rest of the code collapses down to one piece of code for everything:
Although I haven't shown it here, it would also be possible to create variables named
Also note that I've added braces around statements controlled by the
board[x][y], where x and y change, but everything else is identical.I'd rather see (for just one possibility) a small array defining the mapping from input number (1..9) to 2D board position:
board_position positions[10] {
nullptr,
&board[2][0],
&board[2][1],
&board[2][2],
// ...
&board[0][2]
};Using that, the rest of the code collapses down to one piece of code for everything:
auto pos = positions[keyInput];
if (*pos != '*') {
std::cout Graphic.iClearScreen();
obj->Graphic.iPrintBoard(this);
std::cout playerMove(this);
}
else {
*pos = obj->getUserID();
moves++;
}Although I haven't shown it here, it would also be possible to create variables named
x and y (just for example) and compute them based on the user's input. Either way, you end up replacing 9 nearly identical pieces of code with 1.Also note that I've added braces around statements controlled by the
else. As it was, your indentation indicated that you probably wanted the control flow shown above, but since you didn't have braces, the moves++ was not under control of the if/else at all (another problem that clearly needs fixing).Code Snippets
board_position positions[10] {
nullptr,
&board[2][0],
&board[2][1],
&board[2][2],
// ...
&board[0][2]
};auto pos = positions[keyInput];
if (*pos != '*') {
std::cout << "Already taken!\n";
obj->Graphic.iClearScreen();
obj->Graphic.iPrintBoard(this);
std::cout << "Already taken!\n";
obj->playerMove(this);
}
else {
*pos = obj->getUserID();
moves++;
}Context
StackExchange Code Review Q#125423, answer score: 2
Revisions (0)
No revisions yet.