Connect Four - Console Application

I have attempted to make the classic game Connect Four in C++, and I would love some feedback on this project. I have done everything myself without using the help of a tutorial or guide, so sorry if there are some bad practices here.

Main.cpp

// Connect4V2.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include "Board.h"
#include "Player.h"
#include "GameLogic.h"

#include 
#include 

/*
AI
Search for empty blocks with block filled under
Make data structure with these blocks
randomly choose a block from that data structure to spawn on

*/

int main()
{
    Board board;
    Player player1, player2;
    GameLogic gameLogic;

    gameLogic.game(board, player1, player2, gameLogic);

    char c;
    std::cin >> c;
    return 0;
}

GameLogic.h

#pragma once

#include 

class Board;
class Player;
class GameLogic
{
private:
    char m_Turn = X;
    bool m_FoundWinner = false;
    char m_Winner = ' ';

    void allocateFirstTurns(Player& player1, Player& player2, char choice);
    void decideFirstTurn(Player& player1, Player& player2);
    void gameRound(Board& board, Player& player1, Player& player2, GameLogic& gameLogic);
    void changeTurn(char turn);

public:
    static const int ROWS = 9;
    static const int COLUMNS = 9;
    static const int WINNING_ROWS = 4;

    static const char X = 'X';
    static const char O = 'O';
    static const char EMPTY = ' ';

    void game(Board& board, Player& player1, Player& player2, GameLogic& gamelogic);
    void setWinner(char gamePiece);
};

GameLogic.cpp

```
#include "stdafx.h"
#include "GameLogic.h"
#include "Board.h"
#include "Player.h"

void GameLogic::allocateFirstTurns(Player & player1, Player & player2, char choice)
{
if (choice == 'y')
{
player1.setGamePiece(X);
player2.setGamePiece(O);
}

if (choice == 'n')
{
player1.setGamePiece(O);
player2.setGamePiece(X);
}
}

void GameLogic::decideFirstTurn(Player& player1,

Overall, you've done a pretty good job. There's always room for improvement, though...

Compiling with extra warnings

It's always a good idea to compile with as strict a level of compiler warnings as you can. Given the usage of stdafx.h, you're likely using Visual Studio, so this'll be under C/C++ -> General -> Warning Level. I'm using GCC to compile everything, so this is simply -Wall -Wextra. This shows up a few things:

Player.cpp:6:5: warning: unused parameter ‘board’ [-Wunused-parameter]
int Player::getRowPosition(const Board& board) const
^

Player.cpp:23:5: warning: unused parameter ‘board’ [-Wunused-parameter]
int Player::getColPosition(const Board& board) const
^

GameLogic.cpp: In member function ‘void GameLogic::game(Board&, Player&, Player&, GameLogic&)’:
GameLogic.cpp:42:10: warning: unused variable ‘turn’ [-Wunused-variable]
char turn = X; //First turn


These are pretty mild as far as warnings go - nothing that's actually dangerous. However, passing in a parameter that you don't use can be confusing for the person reading your code, so you should try and fix these up.

Stringly-typed functions and reference parameters

In your searchForWinner function, you passing in a std::string searchDirection, which is limited to a few different options. If you ever have a limited number of (distinct) options, an enum (or enum class) is a better choice:

enum class SearchDirection
{
    Horizontal, Vertical, DiagonalLeft, DiagonalRight
};

Passing a boolean reference parameter (bool& foundWinner) is also a bit of a code-smell. Much better would be to return a boolean. This would make your searchForWinner function look like:

bool searchForWinner(GameLogic& gameLogic, char playerPiece, SearchDirection searchDirection);

Your checkForWinner function could then look like:

bool Board::checkForWinner(GameLogic& gameLogic, char playerPiece)
{
    return searchForWinner(gameLogic, playerPiece, SearchDirection::Horizontal) ||
       searchForWinner(gameLogic, playerPiece, SearchDirection::Vertical) ||
       searchForWinner(gameLogic, playerPiece, SearchDirection::DiagonalRight) ||
       searchForWinner(gameLogic, playerPiece, SearchDirection::DiagonalLeft);
}

Code simplifications

initBoard() (which actually has a bug that just hasn't manifested itself because ROWS == COLUMNS currently in your code) can be simplified. std::vector has a constructor that takes a count and a value to initialize with:

void Board::initBoard()
{
    const static std::vector row(GameLogic::COLUMNS, GameLogic::EMPTY);
    m_Board = std::vector>(GameLogic::ROWS, row);
}

You could even do away with this function entirely, and simply do this in the constructor via an initializer-list:

Board::Board()
    : m_Board(GameLogic::ROWS, std::vector(GameLogic::COLUMNS, GameLogic::EMPTY))
{ }

Your getRowPosition and getColPosition have exactly the same code (including the variable being named row in both!). Whenever you have copy-paste code like this, you should refactor it into a different method:

int Player::getPosition(const std::string& which, int size) const
{
    int pos = 0;
    bool positionAllowed = false;
    while (!positionAllowed)
    {
        std::cout > pos;

        if (pos >= 1 && pos < size - 1)
            positionAllowed = true;
        else
            std::cout << "Position out of bounds. Please enter again.\n";
    }
    return pos;
}

You can then change getRowPosition and getColumnPosition to:

int Player::getRowPosition() const
{
    return getPosition("row", GameLogic::ROWS);
}

int Player::getColumnPosition() const
{
    return getPosition("column", GameLogic::COLUMNS);
}

There's a bit of confusion regarding the 0-based vs 1-based coordinates you are using. You try to correct for this initially (but only for rows?) with:

if (row == GameLogic::ROWS - 2 && ...)

This looks far more complicated than need be. You've done your input checking in getRowPosition and getColPosition. I'd simply convert them to the equivalent 0-based array offsets (by subtracting 1), so move then looks like:

row = getRowPosition();
col = getColumnPosition();

if(board.isMoveLegal(row - 1, col - 1)) {
     ...
}

StackExchange Code Review Q#117587, answer score: 2