Connect Four - Console Application

Question

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 <iostream>
#include <vector>

/*
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 <string>

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, Player& player2)
{
    bool chosen = false;
    char choice = ' ';
    while (!chosen)
    {
        std::cout << "Would you like to go first? 'y' - Yes. 'n' - No.\n";
        std::cin >> choice;

        if (choice == 'y' || choice == 'n')
            chosen = true;
    }

    allocateFirstTurns(player1, player2, choice);
}


void GameLogic::game(Board & board, Player& player1, Player& player2, GameLogic& gameLogic)
{

    char turn = X; //First turn
    decideFirstTurn(player1, player2);

    while (!m_FoundWinner)
    {
        gameRound(board, player1, player2, gameLogic);
    }

    board.displayBoard();

    //Announce winner

    std::cout << "Winner: " << m_Winner << "\n";

    if (m_Winner == player1.getGamePiece())
        std::cout << "Player 1 wins the game.\n";

    if (m_Winner == player2.getGamePiece())
        std::cout << "Player2 wins the game.\n";
}

void GameLogic::gameRound(Board & board, Player & player1, Player & player2, GameLogic& gameLogic)
{
    while (!m_FoundWinner)
    {
        if (m_Turn == player1.getGamePiece())
        {
            board.displayBoard();
            std::cout << "Player 1 move.";
            player1.move(board);
            m_FoundWinner = board.checkForWinner(gameLogic, player1.getGamePiece());
            changeTurn(player1.getGamePiece());
        }

        else
        {
            board.displayBoard();
            std::cout << "Player 2 move.";
            player2.move(board);
            m_FoundWinner = board.checkForWinner(gameLogic, player2.getGamePiece());
            changeTurn(player2.getGamePiece());
        }
    }
}

void GameLogic::changeTurn(char turn)
{
    if (turn == X)
        m_Turn = O;

    else
        m_Turn = X;
}

void GameLogic::setWinner(char gamePiece)
{
    m_Winner = gamePiece;
}

Board.h

#pragma once

#include <vector>
#include <iostream>


class GameLogic;
class Board
{
private:
    std::vector<std::vector<char>> m_Board;

    std::string m_SrchHorizontal = "Horizontal";
    std::string m_SrchVertical = "Vertical";
    std::string m_SrchRightDiagonal = "DiagonalRight";
    std::string m_SrchLeftDiagonal = "DiagonalLeft";

    void initBoard();
    void searchForWinner(GameLogic& gameLogic, char playerPiece, std::string searchDirection, bool& foundWinner);

public:
    Board();

    char getBoardPosition(int row, int col) const;

    void displayBoard();
    bool isMoveLegal(int row, int col);
    void addPlayerPiece(int row, int col, char playerPiece);
    bool checkForWinner(GameLogic& gameLogic, char playerPiece);
};

Board.cpp

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

Board::Board()
{
    initBoard();
}

void Board::initBoard()
{
    std::vector<char> tempBoard;

    for (int i = 0; i < GameLogic::ROWS; i++)
    {
        tempBoard.push_back(GameLogic::EMPTY);
    }

    for (int i = 0; i < GameLogic::ROWS; i++)
    {
        m_Board.push_back(tempBoard);
    }

}

void Board::displayBoard()
{
    std::cout << "\n";
    for (int row = 0; row < GameLogic::ROWS - 1; row++)
    {

        std::cout << "\t"; //std::cout << "1 2 3 4 5 6";
        //std::cout << "\nrow";
        for (int col = 0; col < GameLogic::COLUMNS - 1; col++)
        {
            if(col != 0)
                std::cout << "|" << m_Board[row][col] << "|";
        }
        std::cout << "\n";
    }
}

bool Board::isMoveLegal(int row, int col)
{

    if (m_Board[row][col] == GameLogic::EMPTY) //If square player wnats to move to is empty
    {
        if (row == GameLogic::ROWS - 2 && m_Board[row][col] == GameLogic::EMPTY) //If square player wants to move to is on the bottom row.
        {
            return true;
        }

        else
        {
            int tempRow = row;
            tempRow++;
            if (m_Board[tempRow][col] != GameLogic::EMPTY)
            {
                return true;
            }

            else
            {
                std::cout << "You cannot move here.\n";
                return false;
            }

        }
    }

    else
    {
        std::cout << "You cannot move here.\n";
        return false;
    }
}

void Board::addPlayerPiece(int row, int col, char playerPiece)
{
    m_Board[row][col] = playerPiece;

}



char Board::getBoardPosition(int row, int col) const
{
    return m_Board[row][col];
}

bool Board::checkForWinner(GameLogic& gameLogic, char playerPiece)
{
    bool foundWinner = false;

    searchForWinner(gameLogic, playerPiece, m_SrchHorizontal, foundWinner);
    searchForWinner(gameLogic, playerPiece, m_SrchVertical, foundWinner);
    searchForWinner(gameLogic, playerPiece, m_SrchRightDiagonal, foundWinner);
    searchForWinner(gameLogic, playerPiece, m_SrchLeftDiagonal, foundWinner);

    return foundWinner;
}


void Board::searchForWinner(GameLogic& gameLogic, char playerPiece, std::string searchDirection, bool& foundWinner)
{
    if (!foundWinner)
    {
        int i = 0;
        for (int row = 0; row < GameLogic::ROWS; row++)
        {
            for (int col = 0; col < GameLogic::COLUMNS; col++)
            {
                while (m_Board[row][col] == playerPiece && !foundWinner)
                {
                    i++; //Counts blocks 
                    gameLogic.setWinner(playerPiece);

                    if (searchDirection == m_SrchHorizontal)
                        col++;

                    if (searchDirection == m_SrchVertical)
                        row++;

                    if (searchDirection == m_SrchRightDiagonal)
                    {
                        row++;
                        col++;
                    }

                    if (searchDirection == m_SrchLeftDiagonal)
                    {
                        row++;
                        col--;
                    }
                    if (i == GameLogic::WINNING_ROWS)
                    {
                        foundWinner = true;
                    }
                }
                i = 0;
            }
        }
    }
    else
    {
        return;
    }
}

Player.h

#pragma once

class GameLogic;
class Board;
class Player
{
private:
    char m_GamePiece = ' ';
    int m_Score = 0;

    int getRowPosition(const Board& board) const;
    int getColPosition(const Board& board) const;


public:

    void move(Board& board);
    void setGamePiece(const char gamePiece);
    char getGamePiece() const;
};

Player.cpp

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


int Player::getRowPosition(const Board& board) const
{
    int row = 0;
    bool positionAllowed = false;
    while (!positionAllowed)
    {
        std::cout << "Enter row.\n";
        std::cin >> row;

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

int Player::getColPosition(const Board& board) const
{
    int row = 0;
    bool positionAllowed = false;
    while (!positionAllowed)
    {
        std::cout << "Enter column.\n";
        std::cin >> row;

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


void Player::move(Board& board)
{
    int row = 0;
    int col = 0;
    bool moveComplete = false;
    while (!moveComplete)
    {
        row = getRowPosition(board);
        col = getColPosition(board);

        if (board.isMoveLegal(row, col))
        {
            board.addPlayerPiece(row, col, getGamePiece());
            moveComplete = true;
        }
    }

}

void Player::setGamePiece(const char gamePiece)
{
    m_GamePiece = gamePiece;
}

char Player::getGamePiece() const
{
    return m_GamePiece;
}

Answer 1

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<char> row(GameLogic::COLUMNS, GameLogic::EMPTY);
    m_Board = std::vector<std::vector<char>>(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<char>(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 << "Enter " << which << "\n.";
        std::cin >> 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)) {
     ...
}