patterncppModerate
Tic Tac Toe - console application
Viewed 0 times
applicationtoetictacconsole
Problem
I have made a little Tic Tac Toe console application using OOP techniques and would love to get some advice and guidance on what I have done. I have tried to structure the program to be as organized as possible but I am sure there is a lot of room for improvement.
main.cpp
Player.h
```
#pragma once
#include
class Board;
class Player
{
private:
const int winningScore = 1; //Score that player must reach to win game
public:
int GetWinningScore();
bool DecideFirstTurn(Board& board); //Decides who gets the first turn
char PlayerGamePiece(Board&board); //Player gets their game piece
char OpponentGamePiece(Board& board, char player1); //Opponent gets their gamne piece
int PlayerMove(Board& board, const std::vector& playingBoard, int move); //Gets the players movement on board
int PositionOfMove(const std::vector& playingBoard, int high, int low); //Asks player where they would like to move on board
bool CheckGameOver(bool gameOver, int player1Score, int pl
main.cpp
// TicTacToeOOPV2.cpp : Defines the entry point for the console application.
//
#include "stdafx.h"
#include "Board.h"
#include "Player.h"
#include
#include
#include
int main()
{
Board board;
Player player;
std::vector playingBoard(board.GetNumbOfSquares(), board.GetEmpty()); //The playing board
bool gameOver = false;
int move = 0, //Position player to on board
player1Score = 0,
player2Score = 0;
char player1 = player.PlayerGamePiece(board);
char player2 = player.OpponentGamePiece(board, player1); //Player two gets their gamePiece
//Decide first turn
char turn = board.GetGamePieceX(); //Player who is 'X' gets the first turn
board.DisplayBoard(playingBoard); //Display the pplaying board
do
{
while (board.FindWinner(playingBoard) == board.GameOnGoing())
{
//If its player1's turn
if (turn == player1)
{
std::cout > barn;
return 0;
}Player.h
```
#pragma once
#include
class Board;
class Player
{
private:
const int winningScore = 1; //Score that player must reach to win game
public:
int GetWinningScore();
bool DecideFirstTurn(Board& board); //Decides who gets the first turn
char PlayerGamePiece(Board&board); //Player gets their game piece
char OpponentGamePiece(Board& board, char player1); //Opponent gets their gamne piece
int PlayerMove(Board& board, const std::vector& playingBoard, int move); //Gets the players movement on board
int PositionOfMove(const std::vector& playingBoard, int high, int low); //Asks player where they would like to move on board
bool CheckGameOver(bool gameOver, int player1Score, int pl
Solution
First things first: your code is pretty easy to read, and very consistently formatted, good job on that.
You're including the right headers, and didn't go wild with using directives, and that's good too. (Note that
Now on the design of your solution: there's something very strange. You created two classes,
Neither class has any real state - all their members are
So where did all the game state go? Well, it's all in your
That's not a very good design.
Here's a proposal about how to model the game (there are many ways to do it, this is just a rough idea):
The words in italics are supposed to be hints about what properties/methods the classes should have.
The game controller's "round loop" would look this like:
The game controller's "game loop" would:
With that in place, you'll be able to extend your game with AI players for instance. (With a careful design of a Player base class with a virtual
You're including the right headers, and didn't go wild with using directives, and that's good too. (Note that
stdafx.h is a Microsoft-specific thing for precompiled headers I think. It's not portable, and I doubt you need it here. Consider removing it.)Now on the design of your solution: there's something very strange. You created two classes,
Player and Board. With such names, I was expecting them to actually be participants in the game. But looking closer, I see you only ever instantiate one player. That looks real odd for a game that requires two. So, closer look at the classes and here's what jumps out: they are just collections of functions.Neither class has any real state - all their members are
const and might as well be static. (Player's only member doesn't really belong there either - how many games are required to win is a property of the game (or the umpire/referee maybe), but not of the player.)So where did all the game state go? Well, it's all in your
main function. It has the tedious job of maintaining all the game's state by itself, calling into helper functions from Player or Board passing in all the various required bits and pieces of state each time.That's not a very good design.
main does too much bookkeeping, the two classes don't do enough. Your game is going to be hard to extend: try to think of how much you need to change your code if you decide to implement an AI player, and let the user play against it. That's going to be very hard to do with your current code. But it should "only" require creating a new player class with the logic, and a few initialization changes.Here's a proposal about how to model the game (there are many ways to do it, this is just a rough idea):
- A player has a symbol (
XorO) and can pick a move on a board.
- A board knows what moves have been played, can check and record a move, and check the game status (undecided, won, tie).
- The referee/game controller keeps a set of players, tracks their score, owns the board, and runs the game: asks each player in turn to pick a move, gets it recorded if valid, checks for stalemate, etc.
The words in italics are supposed to be hints about what properties/methods the classes should have.
The game controller's "round loop" would look this like:
int position = players[current]->pickMove(board);
if (!board.isValidMove(position)) {
// give players[current] a red card, escort out of the field
} else {
board.set(position, players[current]->symbol());
}
if (board.isTied()) {
// announce a tie, move to next round (i.e. return)
} else if (board.isWon()) {
// announce current player as a winner
// tally score
// move to next round
} else { /* change players */ }The game controller's "game loop" would:
- check if either player has won
- if not, play a round
- back to step one.
- if so, congratulate player and return.
With that in place, you'll be able to extend your game with AI players for instance. (With a careful design of a Player base class with a virtual
pickMove function for instance, and HumanPlayer and Bot subclasses.)Player::DecideFirstTurn is buggy. If the user enters a wrong value, the code flows out of the function without returning. Since it is not a void function, this leads to undefined behavior.Code Snippets
int position = players[current]->pickMove(board);
if (!board.isValidMove(position)) {
// give players[current] a red card, escort out of the field
} else {
board.set(position, players[current]->symbol());
}
if (board.isTied()) {
// announce a tie, move to next round (i.e. return)
} else if (board.isWon()) {
// announce current player as a winner
// tally score
// move to next round
} else { /* change players */ }Context
StackExchange Code Review Q#115116, answer score: 11
Revisions (0)
No revisions yet.