Bug in connect 4 program

[NeilShah]

Hey there.I've just recently gotten into C++, and I've been teaching myself using a couple of books. I thought I was doing alright, but I've spent the past two days trying to figure out what I've been doing wrong with this program, with little success. I've finally decided to give up and desperately ask anyone on this board for help.

I decided to challenge myself by writing a connect 4 program using an array. It's a simple two player program. Turns alternate and the users can input the column that they want to "drop" a chip into on an 8 by 8 board. It will "fall" to the lowest possible spot. Every turn, the program will check for a horizontal and vertical victory condition.

For some reason though, and I think this is a basic problem when I'm either defining or accessing my array, the program malfunctions whenever I "drop" apiece into column 1 or column 8. In column one (or 0 in the array), the height function doesn't work, and the piece falls through until the bottom every time. In column 8, the display also changes column 1 after the first piece.

Some basic notes on my program. The array is a 8 by 7 grid, and the last row is to maintain how "tall" the current column is.

If someone would plug this into their complier and figure out what I've done wrong, I would be eternally indebted to you.

Any other comments on this program would be very much appreciated. Thanks for any help you can give me!

#include <cstdio>
#include <cstdlib>
#include <iostream>
using namespace std;

//globalvariable declarations
int board [8][7] = {0, 0}; // make everything equal 0
// NOTE: The 8th row is the row that contains the height of the column
int playerMove = 1; // which player's move it is.
int victory; // who has won (0 if no one)

// prototype declarations
void displayBoard ();
int inputNewMove ();
int checkForWin ();
int gameOver ();
int main ()
{
displayBoard ();
for (int i = 0 ; i < 15 ; i++)
{
cout << "Player " << playerMove << "'s turn." << endl;
cout << board[8][1] << endl;
cout << board[7][1] << endl;

inputNewMove ();
displayBoard ();
victory = checkForWin ();
if (victory > 0)
{
gameOver ();
return 0;
}
}

system("PAUSE");
return 0;
}



void displayBoard ()
{
cout << " 1 2 3 4 5 6 7 8" << endl;
cout << " ---------------";
for (int boardRow = 0 ; boardRow<=7 ; boardRow++)// cycle through rows
{
cout << endl; // go to the next line
cout << (boardRow + 1) << "|";
for (int boardColumn = 0 ; boardColumn<=7 ; boardColumn++) // cycle through columns
{
if (board [boardRow][boardColumn] == 0) // empty
{
cout << " ";
}
if (board [boardRow][boardColumn] == 1) // player 1's piece
{
cout << "1 ";
}
if (board [boardRow][boardColumn] == 2) // player 2's piece
{
cout << "2 ";
}
}
}
cout << endl << endl;
return;
}

int inputNewMove()
{
int x;
int height;

for (;;)
{
cout << "Column? ";
cin >> x;

if ((x <= 8) && (x >= 1) && (board[8][x-1] != 8))
{
break;
}

else
{
cout << "Choice must be between 1 and 8, and column cannot be full." << endl;
}
}


board[8][x-1]++; // adds one to the HEIGHT OF THE COLUMN VALUE (ROW 8)
height = (8 - (board[8][x-1])); // determines point in array to fall (simulates the falling piece until it stops)
board [height][x-1] = playerMove; // sets array coordinate to the number of the player


//switch move
if (playerMove == 1)
playerMove++;
else
playerMove--;

return 0;
}


int checkForWin ()
{

//test horizontal victory
for (int i=0; i<=4; i++) // columns
{
for (int j=0; j<=8; j++) // rows
{

if (board[j][i] == 1 && board[j][i+1] == 1 && board[j][i+2] == 1 && board[j][i+3] == 1)
{
return 1;
}
if (board[j][i] == 2 && board[j][i+1] == 2 && board[j][i+2] == 2 && board[j][i+3] == 2)
{
return 2;
}
}
}

//test vertical victory
for (int i=0; i<=8; i++) // columns
{
for (int j=0; j<=4; j++) // rows
{

if (board[j][i] == 1 && board[j+1][i] == 1 && board[j+1][i] == 1 && board[j+1][i] == 1)
{
return 1;
}
if (board[j][i] == 2 && board[j+1][i] == 2 && board[j+2][i] == 2 && board[j+3][i] == 2)
{
return 2;
}
}
}


return 0;
}


int gameOver()
{
cout << endl << "Game Over! Player " << victory << "has won!" ;
system("PAUSE");
}

[Duthomhas]

I hope the complete lack of indentation and spacing errors in the output are simply due to the way you copied and pasted your code...

If you have an array of 8 elements, there is no element 8.

  int fooey[8];
  fooey[8] = 12;  // <-- ERROR 

Unfortunately, a lot of C++ compilers don't do bounds-checking at compile-time, but you can tell them to insert bounds-checking code when compiling. For example:

g++ -Wall -pedantic -fbounds-check Connect4.cpp -o Connect4

Then, when you run your program and it tries to access board[8] your program will crash telling you that you've tried to access an invalid element, and you can fix it.

A related problem is that you keep assuming the size of the board throughout the program. Instead, use consts to define the size of your array, and refer to them when you want to loop through its elements.

Your next problem is that you kept mixing up the row and column indices into your gameboard array. For example, in checkForWin() your comment says "test horizontal victory" but you are actually checking for vertical victories.

The idea to have an extra array of ints specifying the current height of a row is very interesting. Unfortunately it seems to have caused you the most grief. A better way is to just check to see whether the topmost row of the column has a piece in it or not.

Lastly, system("PAUSE") is evil. Please avoid it.

I hope you don't mind, but I've taken the liberty of modifying your code to implement some improvements and fix your bounds error:

#include <cstdio>
#include <cstdlib>
#include <iostream>
#include <limits>
using namespace std;

// standard Connect Four board size
const int NROWS = 6;
const int NCOLS = 7;

//globalvariable declarations
int board [NROWS][NCOLS] = {0, 0}; // make everything equal 0
// NOTE: The 8th row is the row that contains the height of the column

// I see what you are trying to do above.
//   A better way is just to check to see if their is a piece in the
//   topmost row of the column.
// Since the 8th row is messing you up, I've removed it.

int playerMove = 1; // which player's move it is.
int victory; // who has won (0 if no one)

// prototype declarations
void displayBoard ();
int inputNewMove ();
int checkForWin ();
void gameOver ();

//----------------------------------------------------------------------------
void pause()
  {
  // Please don't use system("PAUSE")
  cin.seekg( 0, ios::end );
  cin.clear();
  cout << "Press ENTER to continue..." << flush;
  cin.ignore( numeric_limits<streamsize>::max(), '\n' );
  }

//----------------------------------------------------------------------------
int main ()
  {
  displayBoard ();
  for (int i = 0 ; i < 15 ; i++)
    {
    cout << "Player " << playerMove << "'s turn." << endl;
//    cout << board[8][1] << endl;  I presume this is debug info
//    cout << board[7][1] << endl;  as the user won't care to see it.
//                                  (besides which, I killed board[7]...)

    inputNewMove ();
    displayBoard ();
    victory = checkForWin ();
    if (victory > 0)
      {
      gameOver ();
      return 0;
      }
    }

  pause();
  return 0;
  }

//----------------------------------------------------------------------------
void displayBoard ()
  {
  // suggestion: use a lookup table
  const char LUT[] = " 12";

  cout << "  1 2 3 4 5 6 7 8" << endl;
  cout << "  ---------------";
  for (int boardRow = 0 ; boardRow<NROWS ; boardRow++)// cycle through rows
    {
    cout << endl; // go to the next line
    cout << (boardRow + 1) << "|";
    for (int boardColumn = 0 ; boardColumn<NCOLS ; boardColumn++) // cycle through columns
      {
      cout << LUT[ board[boardRow][boardColumn] ] << ' ';
/*
      if (board [boardRow][boardColumn] == 0) // empty
        {
        cout << "  ";
        }
      if (board [boardRow][boardColumn] == 1) // player 1's piece
        {
        cout << "1 ";
        }
      if (board [boardRow][boardColumn] == 2) // player 2's piece
        {
        cout << "2 ";
        }
*/
      }
    }
  cout << endl << endl;
  return;
  }

//----------------------------------------------------------------------------
bool isColumnFull( int col )
  {
  return board[0][col] != 0;
  }

//----------------------------------------------------------------------------
int inputNewMove()
  {
  int x;
  int height;

  for (;;)
    {
    cout << "Column? ";
    cin >> x;

    if ((x <= 8) && (x >= 1) && !isColumnFull( x-1 ))  
      {                       //(board[8][x-1] != 8))
      break;
      }

    else
      {
      cout << "Choice must be between 1 and 8, and column cannot be full." << endl;
      }
    }


// brilliant! however, since board[7] no longer exists... 
//  board[8][x-1]++; // adds one to the HEIGHT OF THE COLUMN VALUE (ROW 8)
//  height = (8 - (board[8][x-1])); // determines point in array to fall (simulates the falling piece until it stops)

  // search for the next available space
  for (height=NROWS-1; board[height][x-1] != 0; height--);

  board [height][x-1] = playerMove; // sets array coordinate to the number of the player


  //switch move
  if (playerMove == 1)
    playerMove++;
  else
    playerMove--;

  return 0;
  }


//----------------------------------------------------------------------------
int checkForWin ()
  {

  //test for VERTICAL victory
  for (int i=0; i<NCOLS-3; i++) // columns
    {
    for (int j=0; j<NROWS; j++) // rows
      {

      if (board[j][i] == 1 && board[j][i+1] == 1 && board[j][i+2] == 1 && board[j][i+3] == 1)
        {
        return 1;
        }
      if (board[j][i] == 2 && board[j][i+1] == 2 && board[j][i+2] == 2 && board[j][i+3] == 2)
        {
        return 2;
        }
      }
    }

  //test HORIZONTAL victory
  for (int i=0; i<NCOLS; i++) // columns
    {
    for (int j=0; j<NROWS-3; j++) // rows
      {

      if (board[j][i] == 1 && board[j+1][i] == 1 && board[j+1][i] == 1 && board[j+1][i] == 1)
        {
        return 1;
        }
      if (board[j][i] == 2 && board[j+1][i] == 2 && board[j+2][i] == 2 && board[j+3][i] == 2)
        {
        return 2;
        }
      }
    }

  // don't forget to test for diagonal victory
  // (combine what was done for your vertical and horizontal tests)


  return 0;
  }


//----------------------------------------------------------------------------
void gameOver()
  {
  cout << endl << "Game Over! Player " << victory << "has won!\n" ;
  pause();
  }

Hope this helps.

[edit] Oh yeah. You've done a very good job for just learning it. Very impressive!

[jsmith]

I only looked at the code, but I can see that you have some array indexing problems.

board is declared as

int board[8][7];

which means the valid indices are [ 0-7 ][ 0-6 ].

However, in inputNewMove() you access board[8][x-1], and 8 is out of bounds.

Also, in checkForWin(), your outer for() loop goes from 0 ... 8, again out of bounds.

When I'm writing code like this, I find that one of my most common mistakes is to index [column][row] in some places and [row][column] in others. I did not check to see if you did that here; you might want to double check that too.

[Faldrax]

Duoas wrote

Lastly, system("PAUSE") is evil. Please avoid it.

To clarify.
Using the system() function is a bad habit to get into as it makes the code platform dependant and can open security holes.
IMHO it is fine for short test code - I will often stick a system("pause") at the end of main() when I want to verify a code fragment - BUT only in code I know I will then throw away, and NEVER in production code.

[NeilShah]

Thanks so much! This helps a lot. I'll clean up my code tonight and I'll see if I have any more questions. I really appreciate this, guys.