OOP chess game implementation in C++

Question

I am attempting one of the questions from Cracking the Coding Interview:

"Design a chess game using object oriented principles."

The solutions are in Java and below is my attempt to create something similar in C++. This has brought up a few issues and gaps in my understanding. The below code compiles and gives me the desired logic however I know there is plenty of room for improvement, hence the post. In particular, I am looking for a nice way of removing the ChessPieces created in setup(). I am aware this may have something to do with ownership problems as I am trying to delete something from outside of the scope in which it was created, however I like the organisation of one method whose purpose it is to create and another whose purpose is to destroy.

#include <iostream>
#include <algorithm>
#include <string>
#include <vector>
class ChessPiece {
 public:
 const std::string position;
 const std::string piece;
 bool is_captured;
 ChessPiece(std::string position, std::string piece, bool is_captured) :
 position(position), piece(piece), is_captured(is_captured) {};
};
class PlayerBase {
 public:
 bool is_checked = false; 
 std::vector<const ChessPiece*> pieces;
 char colour;
 void print_pieces() {
 for(auto &cp : pieces) {
 std::cout << cp->piece << " is_alive "
 << cp->is_captured << std::endl;
 }
 }
 /* 
 * Add a ChessPiece to pieces as necessary.
 */ 
 void add_pieces(const ChessPiece*);
 /* 
 * Remove a ChessPiece to pieces as necessary.
 */ 
 void remove_pieces(const ChessPiece&);
 /*
 * Process a player's desired next move (includes checks if it is a
 * valid move and will call and erase methods if necessary
 */
 virtual ChessPiece next_move(ChessPiece cp)
 {
 return cp;
 };
};
void PlayerBase::add_pieces(const ChessPiece *cp) {
 pieces.push_back(cp);
};
void PlayerBase::remove_pieces(const ChessPiece& cp) {
 /* remove description:
 * Transforms the range [first,last) into a range with all the elements 
 * that compare equal to val removed, and returns an iterator to the new end
 * of that range.
 */
 auto it = std::remove(pieces.begin(), pieces.end(), &cp);
 pieces.erase(it , pieces.end());
};
class HumanPlayer : PlayerBase {
 /*
 * Here next_move is taken as an input
 */
 ChessPiece next_move(ChessPiece cp) { return cp; };
};
class Computer : PlayerBase {
 ChessPiece move;
 /*
 * Computer calculates an appropriate move depending on difficulty rating
 */
 ChessPiece next_move() { return move; };
};
class Player_1 : public PlayerBase {
 public:
 char colour = 'W';
};
class Player_2 : public PlayerBase {
 public:
 char colour = 'B';
};
class GameManager {
 public:
 ChessPiece process_turn(PlayerBase player) 
 {
 std::cout << "processing " << player.colour << std::endl;
 /*
 * Here the player would input a possible move for a chess piece
 */
 ChessPiece cp("foo", "bar", false);
 return cp;
 };
 bool accept_turn(ChessPiece turn)
 { 
 /* Perform checks on chesspiece returned by process_turn() above.
 * Return true if move is valid
 */
 return true; 
 };
 /*
 * Setup the game
 */
 void setup(PlayerBase&, PlayerBase&);
 void teardown(PlayerBase&, PlayerBase&);
};
void GameManager::setup(PlayerBase& p1, PlayerBase& p2) {
 /*
 * Here we prepare the chess board. For example by creating 8 pawns
 */
 for(int i = 0; i < 16; i++) {
 if(i < 8) {
 ChessPiece *p = new ChessPiece("a1", "Pawn", false);
 p1.add_pieces(p);
 }
 }
}
void GameManager::teardown(PlayerBase& p1, PlayerBase& p2) {
 //delete ChessPieces created in setup()
};
int main()
{
 Player_1 p1;
 Player_2 p2;
 GameManager game;
 game.setup(p1, p2);
 p1.print_pieces();
 // while(!p1.is_checked && !p2.is_checked) {
 // ChessPiece move_to_make = game.process_turn(p1);
 // game.accept_turn(move_to_make);
 // move_to_make = game.process_turn(p1);
 // game.accept_turn(move_to_make);
 // }
 game.teardown(p1, p2);
 return 0;
}

Answer 1

My suggestions:

Use a namespace to define the classes/functions for the game

Using namespaces is good way to contain all the classes and functions that help define the game.

namespace Chess
{
 class ChessPiece {
 ...
 };
 class PlayerBase {
 ...
 };
 class HumanPlayer : PlayerBase {
 ...
 };
 class Computer : PlayerBase {
 ...
 };
 class Player_1 : public PlayerBase {
 ...
 };
 class Player_2 : public PlayerBase {
 ...
 };
 class GameManager {
 ...
 };
}

and then use:

using namespace Chess;
int main()
{
 Player_1 p1;
 Player_2 p2;
 GameManager game;
 game.setup(p1, p2);
 p1.print_pieces();
 game.teardown(p1, p2);
 return 0;
}

to test the code.

Change ChessPiece to just Piece after the namespace has been introduced

The Chess part of ChessPiece becomes redundant since Chess is now a namespace.

namespace Chess
{
 class Piece {
 ...
 };

Change PlayerBase to just Player

Including Base in the class name does not add any more meaning to the type than simply using Player.

With that change, you will have:

 class HumanPlayer : Player {
 ...
 };
 class Computer : Player {
 ...
 };
 class Player_1 : public Player {
 ...
 };
 class Player_2 : public Player {
 ...
 };

Missing public while inheriting HumanPlayer and Computer

I am guessing that is an oversight and you meant to use:

 class HumanPlayer : public Player {
 ...
 };
 class Computer : public Player {
 ...
 };

Non-symmetric class names

You have HumanPlayer and Computer. You should have either Human and Computer or HumanPlayer and ComputerPlayer. I think it makes more sense to change Computer to ComputerPlayer rather than change HumanPlayer to Human.

Duplicate member variable colour

You have colour as a member variable in Player as well as in Player_1 and Player_2. You should:

1. Remove the member variables from Player_1 and Player_2.
2. Set the value of colour appropriately in the constructor.

Add a constructor to Player.

 Player(char colour = 'W') : colour(colour) {}

and then update Player_1 and Player_2 to:

 class Player_1 : public Player {
 public:
 Player_1() : Player('W') {}
 };
 class Player_2 : public Player {
 public:
 Player_2() : Player('B') {}
 };

• I'll add comments tomorrow: One question: isn't using namespace Chess; bad for the same reasons using namespace std; is bad?
• R Sahu: It is bad if used in a .h file. It's fine in a .cpp file.
• Roland Illig: Using your own namespace is ok since you know which names it contains. The difference with the std namespace is that it contains hundreds of names, most of them everyday words.

Answer 2

Here are things to consider:

A chess piece in of itself has not state. It does not know where it is, it only knows what it is called and how it moves. It does not know if it is captured or anything of that sort. So the logic for its position and captured is probably not appropriate for it.

Then, who tracks the position of the game? It's the board. The board concept is missing. Game manager only manages who plays, turns, checks, etc. Board manages pieces on it. Player manage their names, info, etc.

Just suggestions.

• laker93: I see your point. Although my feeling is that it's possible to personify the chess pieces so that each one has an awareness of its state of play and position. I guess this has the disadvantage that in order for a computer to assess the next move it has to iterate through all ChessPieces, whether they are in play or not. The advantage of your suggestion is that the computer only has to analyse the contents of BoardManager. Another advantage is that it separates out the initial setup logic of creating ChessPieces from the game logic in to two distinct classes. All in all, you make a good point