TicTacToe game with large switch statements

Question

This 5 classes: 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 << "keyInput " << keyInput << std::endl;
 switch(keyInput)
 {
 case (1):
 if (board[2][0] != '*')
 {
 std::cout << "Already take!\n";
 obj->Graphic.iClearScreen();
 obj->Graphic.iPrintBoard(this);
 std::cout << "Already take!\n";
 obj->playerMove(this);
 }
 else
 board[2][0] = obj->getUserID();
 moves++;
 //std::cout << "first one here eh mate! " << std::endl;
 //std::cout << this->board[2][0] << std::endl;
 break;
 case (2):
 if (board[2][1] != '*')
 {
 std::cout << "Already take!\n";
 obj->Graphic.iClearScreen();
 obj->Graphic.iPrintBoard(this);
 std::cout << "Already take!\n";
 obj->playerMove(this);
 }
 else
 board[2][1] = obj->getUserID();
 moves++;
 break;
 case (3):
 if (board[2][2] != '*')
 {
 std::cout << "Already take!\n";
 obj->Graphic.iClearScreen();
 obj->Graphic.iPrintBoard(this);
 std::cout << "Already take!\n";
 obj->playerMove(this);
 }
 else
 board[2][2] = obj->getUserID();
 moves++;
 break;
 case (4):
 if (board[1][0] != '*')
 {
 std::cout << "Already take!\n";
 obj->Graphic.iClearScreen();
 obj->Graphic.iPrintBoard(this);
 std::cout << "Already take!\n";
 obj->playerMove(this);
 }
 else
 board[1][0] = obj->getUserID();
 moves++;
 break;
 case (5):
 if (board[1][1] != '*')
 {
 std::cout << "Already take!\n";
 obj->Graphic.iClearScreen();
 obj->Graphic.iPrintBoard(this);
 std::cout << "Already take!\n";
 obj->playerMove(this);
 }
 else
 board[1][1] = obj->getUserID();
 moves++;
 break; 
 case (6):
 if (board[1][2] != '*')
 {
 std::cout << "Already take!\n";
 obj->Graphic.iClearScreen();
 obj->Graphic.iPrintBoard(this);
 std::cout << "Already take!\n";
 obj->playerMove(this);
 }
 else
 board[1][2] = obj->getUserID();
 moves++;
 break; 
 case (7):
 if (board[0][0] != '*')
 {
 std::cout << "Already take!\n";
 obj->Graphic.iClearScreen();
 obj->Graphic.iPrintBoard(this);
 std::cout << "Already take!\n";
 obj->playerMove(this);
 }
 else
 board[0][0] = obj->getUserID();
 moves++;
 break;
 case (8):
 if (board[0][1] != '*')
 {
 std::cout << "Already take!\n";
 obj->Graphic.iClearScreen();
 obj->Graphic.iPrintBoard(this);
 std::cout << "Already take!\n";
 obj->playerMove(this);
 }
 else
 board[0][1] = obj->getUserID();
 moves++;
 break;
 case (9):
 if (board[0][2] != '*')
 {
 std::cout << "Already take!\n";
 obj->Graphic.iClearScreen();
 obj->Graphic.iPrintBoard(this);
 std::cout << "Already take!\n";
 obj->playerMove(this);
 }
 else
 board[0][2] = obj->getUserID();
 moves++;
 break;
 }

• Dan Oberlam: Please either include the code to be reviewed either in its entirety, or the part you're most interested in having reviewed.
• justthom8: Nice! Edit my post to say whatever you feel like it should say.

Answer 1

The code you've posted is highly repetitive. You have 9 repetitions of nearly identical code, with a couple of instances of 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 << "Already taken!\n";
 obj->Graphic.iClearScreen();
 obj->Graphic.iPrintBoard(this);
 std::cout << "Already taken!\n";
 obj->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).

• justthom8: I don't understand your first bit of code. Can you explain it a little bit. It may be my inexperience, but I don't understand having what looks like an array, positions[10] with a board_position type.
• Jerry Coffin: @justthom8: You don't have code here to tell me board's type, so I was using board_position as an alias for that unknown type. Let's say your board is something like char array[3][3];. In this case, board_position would be char *.
• justthom8: Ah, I see. What about positions[10]? That's where I got confused. My compiler complained that I couldn't use that kind of initialization unless I enabled C++11 flag. Sorry for being a noob, but it's hard to google very specific questions like this.
• Jerry Coffin: There is no positions[10]. When you define an array like int array[10], it means that the legal subscripts are array[0] through array[9]; (I defined it as an array of 10 because you were using 1 through 9, and it seemed easiest to just use that, and basically ignore positions[0]).
• justthom8: Ohhh, so you mean to put an equal sign there. That makes far more sense than some kind of class definition, or new C++ feature I don't know.