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)) {
 ...
}

• Ryan Swann: This is fantastic! It addressed everything that I was concerned about with my code and more. Thank you so much.
• Ryan Swann: Hello there. After adding everything that you suggested it seems to me there is a bug within the code. When checking for a winner I am now returning true or false but it doesn't stay in the while loop when doing so. I am now returning true when 'i == winningRows' but this causes a bug that doesn't actually count every gamePiece available. So in essence; using a void function and passing as a bool ref works but returning as true doesn't. Thank you for your time.
• Ryan Swann: Ah ha! I have solved the bug. I wasn't returning anything other than true and it caused my program to error in this way.