2 player chess game in C++

I have written a 2 player chess game in C++.

Things I have accomplished so far:

• Legal moves validation

• Bit board generation

Things I look forward to (would appreciate advice on them):

• Move generation

• Move ordering (based on move values for each square)

• Pruning

• Reading data from opening books

Here is the model breakdown:

• GameLogic: As the name indicates, initializes two players and keeps track of current player. Takes turns etc.

• Player: Contains 16 pieces, checks for check and checkmate, castling, etc.

• Piece: Contains respective piece strategies. Responsible for moving piece logic.

• PieceStrategy: Responsible for respective move validation and bit board generation.

• Bitboard: Contains 64 bit integers for possible moves and current positions on the board.

• Board:*Contains 8x8 arrays for positions and moves representation.

Bitboard:

```
class Bitboard {

public:
U64 position;
U64 moves;

LinkedList *computePositionsFromBitboard(U64 bitboardPosition) {

LinkedList *list = new LinkedList();

for(int order = 0; bitboardPosition != 0;) {
for(; (bitboardPosition & 1) != 1; bitboardPosition >>= 1) {
order ++;
}

bitboardPosition &= 0b1111111111111111111111111111111111111111111111111111111111111110;

int x = order % 8;
int y = order / 8;

Position position = Board::indexToPosition(x, y);
list->add(position);
}

return list;
}

U64 computeBitboardFromPosition(Position position) {

Index index = Board::positionToIndex(position);
int x = index.x;
int y = index.y;

int order = (y * 8) + x;
U64 bitboard = 1ull position |= second->position;
}

void performBitwiseANDForPosition(Bitboard first, Bitboard second) {
first->positi

There's a lot of code here, so I haven't gone through all of it.
A few remarks on things that caught my eye though:

Repetition

I see a lot of repeated or very similar code in the Board class and board
updating in updatePresenceAndMovesArrays(). Don't Repeat Yourself
is one of the golden rules of programming.

In Board, we have this:

bool whitePresenceArray[8][8];
   bool blackPresenceArray[8][8];

   bool whitePawnPresenceArray[8][8];
   bool whiteKnightPresenceArray[8][8];
   bool whiteBishopPresenceArray[8][8];
   bool whiteRookPresenceArray[8][8];
   bool whiteQueenPresenceArray[8][8];
   bool whiteKingPresenceArray[8][8];

   bool blackPawnPresenceArray[8][8];
   bool blackKnightPresenceArray[8][8];
   bool blackBishopPresenceArray[8][8];
   bool blackRookPresenceArray[8][8];
   bool blackQueenPresenceArray[8][8];
   bool blackKingPresenceArray[8][8];

And then following the same set of small matrices for the Moves. You should turn that into a helper structure:

struct PieceInBoard // Probably not a great name, just an example
{
    bool pawn[8][8];
    bool knight[8][8];
    bool bishop[8][8];
    bool rook[8][8];
    bool queen[8][8];
    bool king[8][8];
};

class Board
{
    PieceInBoard blackPresence;
    PieceInBoard blackMoves;

    PieceInBoard whitePresence;
    PieceInBoard whiteMoves;
};

Likewise, those massive memset blocks that zero fill the arrays can now be replaced by a single memset on the PieceInBoard instance, or even better be moved into a member method of PieceInBoard (such as a clear() method like it is common in the Standard Library).

Try to look into some of your loops as well. I can see several places that look like copy-paste only changing one or two things. Try to refactor these cases into individual helper functions.

Use C++ references

I see a lot of pointers in your code. Pointers are normally associated with three concepts:

• The pointer can be null, usually to indicate an optional parameter.

• The pointer is actually the head of a C-style array.

• You are passing ownership of some memory to another object/function.

Apart from that, when you just want to pass a reference to a single object, which shouldn't be null, use a C++ reference. When neither of the points above fit your intent, a reference is much more clear and safer and almost certainly the best choice.

Still on pointers though...

Who manages the memory?

I can see a few calls to operator new in there, but no calls to delete. In C++, you must manually free memory that you allocate, or use automated memory management with smart pointers.

Using smart pointers would be the recommended in most cases, unless you have a very good reason to take matters in your own hands an risk running into memory leaks and other nasty memory corruption bugs that we easily run into when doing that sort of stuff.

Other minor issues

-
Please don't this-> qualify class members. We don't do that in C++. If you'd like to have a visual distinction between members and non-members, consider a prefix such as m_, m_likeThis, or a suffix _, likeThis_. But keep in mind that adding prefixes according to context can be a nuisance when you need to refactor some non-member to member or vice-versa. Potentially lots of places to update. Same issue with using this->.

-
C-style cast on class instance is very dangerous. See: "The dangers of C-style casts".

((AIPlayer *)this->currentPlayer)->generateMove();

Use either static_cast if you know for sure that currentPlayer is in fact an AIPlayer instance. Otherwise, use dynamic_cast and check the return value for nullptr before dereferencing it.

-
Use auto when you have to repeat a type on both sides of an expression. It was invented to avoid things like this:

LinkedList *movesList = new LinkedList();

Now you can write the type on the right-hand side only:

auto movesList = new LinkedList();

-
You use the literal characters 1, 8, a, g, etc in some places. I'd prefer to see an enum with more descriptive names instead.

StackExchange Code Review Q#124394, answer score: 3