[Absolute.]

Hello all, I just joined this site seeing as I have started my journey in school to become a programmer and I'm sure it is nice to bounce ideas and questions off of people on the forums from time to time. I don't have too much experience programming, ~ 1 year off and on of C++/Java/VB.

A little background on the problem I'm encountering with my program. In this program the user enters the column number that they wish to play in and I am having problems on the error checking part. I had the normal "out of bounds" checking fine, but when I tried to add in the "if column is full" check I ran into an error. My logic could be wrong, or maybe I am missing something because I'm tired

--- I've narrowed it down to the check full function not increasing 'A' for some reason in my for loop, I THINK

Get column number function:

// This function receives a column from the user
// checks the validity and then places the mark
void getCol(char boardArray[6][7], char playerMark) 
{
	int a = 6;
	int col = 0;
	
	// Asks for colmn number
	cout << "Choose a colmn: ";
	cin >> col;

	// Data check
	while (col < 0 || col > 6 || checkCol(boardArray, playerMark, col) == true)
	{
		cout << "Invalid Input!" << endl;
		cout << "Choose a colmn: ";
		cin >> col;
	}
	// Loops through all colmns
	for (int b = 0; b < 7; b++)
		// Checks colmn chosen
		if (col == B)/>
		{
			while (boardArray[a][b] != '-')
				a--;
			boardArray[a][b] = playerMark;
		}
}

Check if column is full function:

// This function checks to see if the column is full and 
// the the column number matches. If it does true is returned.
bool checkCol(char boardArray[6][7], char playerMark, int col)
{
	for (int a = 0; a < 7; a++)
	{
		cout << "COL:    " << col << endl;
		cout << "A  :    " << a << endl;
		if (boardArray[0][a] == playerMark && col == a)
		{
			cout << "TRUE" << endl;
			return true;
		}
		else
		{
			cout << "FALSE" << endl;
			return false;
		}
	}
}

[jimblumberg]

The first thing I see is a lot of "magic" numbers. You should replace those "magic" numbers with const variables with meaningful names ( the 7 in the following snippet).

    for (int a = 0; a < 7; a++)

Next Insure you are not accessing the arrays out of bounds. Remember in C/C++ arrays start at 0 and end at size - 1.
In the following snippet:

void getCol(char boardArray[6][7], char playerMark) 
{
	int a = 6;

for (int b = 0; b < 7; b++)
		// Checks colmn chosen
		if (col == b )
		{
			while (boardArray[a][b] != '-')

In the while the first time you go through the loop boardArray[a][b] is accessed out of bounds (a == 6).

Also you should try to use meaningful variable names throughout your program. Instead of single letter variable names make your names mean something within the content.

Jim

This post has been edited by jimblumberg: 06 August 2011 - 07:48 PM

[Absolute.]

Quote

The first thing I see is a lot of "magic" numbers. You should replace those "magic" numbers with const variables with meaningful names ( the 7 in the following snippet).

I have been trying to get past this and use constants more often, I realize how much better they are but sometimes just don't use them for some reason.

Quote

Next Insure you are not accessing the arrays out of bounds. Remember in C/C++ arrays start at 0 and end at size - 1.

Also you should try to use meaningful variable names throughout your program. Instead of single letter variable names make your names mean something within the content.

Thanks, my instructor has always just used 1 letter variable names in for loops and I guess I picked up the bad habit.


-------------------------------------------------------------------------------------------


I have tried to fix most of the things u mentioned and made it a little more helper friendly, but I am now having some trouble with placing the playerMark in the right column. It seems like column 0 and 6 are overlapping somewhere and I can't find it anywhere. Could someone take a look and see if they could help? Thanks :]

/*
Conenct Four
By: Matthew Lepley
08/2011
*/

#include <iostream>

using namespace std;

// CONSTANTS
const int ROWS = 5;
const int COLS = 6;

// Prototypes
void getCol(char boardArray[ROWS][COLS], char playerMark);
void showBoard(char boardArray[ROWS][COLS]);
void clearBoard(char boardArray[ROWS][COLS]);

int main() {
 
	// Variables
	char boardArray[ROWS][COLS];
	char playerMark = 'X';
	bool test = false;

	// Initialize board
	clearBoard(boardArray);
	showBoard(boardArray);

	while (test == false) {				// TEMP loop for testing
	getCol(boardArray, playerMark);
	showBoard(boardArray);
	}
	cin >> boardArray[0][0];			// TEMP pause for testing

	return 0;
}

// This function receives a column from the user
// checks the validity and then places the mark
void getCol(char boardArray[ROWS][COLS], char playerMark) 
{
	int column;
	
	// Asks for column number
	cout << "Choose a column: ";
	cin >> column;

	// Data check for column number
	while (column < 0 || column > 6)
	{
		cout << "Invalid Input!" << endl;
		cout << "Choose a column: " << endl;
		cin >> column;
	}

	int row = ROWS + 1;				// Sets row to 6
	// Loops through all columns
	for (int col = 0; col < COLS + 1; col++)
		// Checks column chosen
		if (col == column)
		{
			while (boardArray[row][col] != '-')
				row--;
			boardArray[row][col] = playerMark;
		}
}

// This function displays the current board
void showBoard(char boardArray[ROWS][COLS])
{
	cout << "         CONNECT FOUR       " << endl;
	cout << "  --------------------------- " << endl;
	cout << "   0   1   2   3   4   5   6" << endl;
	cout << "  --------------------------- " << endl;
	for (int rows = 0; rows < ROWS + 1; rows++)
	{
		for (int cols = 0; cols < COLS + 1; cols++)
			cout << " | " << boardArray[rows][cols];
		cout << " | \n";
	}
	cout << "  --------------------------- " << endl;
}

// This function clears the board and sets all locs to '-'
void clearBoard(char boardArray[ROWS][COLS])
{
	for (int rows = 0; rows < ROWS + 1; rows++)
		for (int cols = 0; cols < COLS + 1; cols++)
			boardArray[rows][cols] = '-';

}

[jimblumberg]

In the following snippet:

// CONSTANTS
const int ROWS = 5;
const int COLS = 6;


void getCol(char boardArray[ROWS][COLS], char playerMark) 
{
//.............
        
	int row = ROWS + 1;				// Sets row to 6
	// Loops through all columns
	for (int col = 0; col < COLS + 1; col++)
		// Checks column chosen
		if (col == column)
		{
			while (boardArray[row][col] != '-')
				row--;
			boardArray[row][col] = playerMark;
		}
}

You have an array boardArray[ROWS][COLS] with the sizes of ROWS == 5 and COLS == 6. You then try to access this array with an index of 6 and 7. This is accessing your array out of bounds. Remember that arrays start at 0 and end at size - 1 so in this case you can only access elements ROWS 0 - 4 and COLS 0 - 5.
So trying to access boardArray[5][6] is accessing the arrays out of bounds. You must never exceed size - 1 for your array index.

Your new formatting and variable names make this program much easier to follow your logic.

Jim

This post has been edited by jimblumberg: 07 August 2011 - 10:27 AM

[Absolute.]

Thank you, I don't know why I set my constants to 5 and 6, maybe i was thinking of the index number of the array since i was wanting a 6x7 board.

[jimblumberg]

The problem is accessing the array out of bounds, not the size of your constant. In the following snippet I could change the 5 to 49 and it your program would still be accessing your array out of bounds. Look at the following snippet:

const int ROWS = 5;
   boardArray[ROWS][COLS]
   int row = ROWS + 1; 
   while (boardArray[row][col] != '-')

That last line is accessing the array out of bounds. Even if you removed the + 1 it would be accessing the array out of bounds. Arrays start at 0 and stop at size - 1, not at size or size + anything.

Jim

This post has been edited by jimblumberg: 07 August 2011 - 11:16 AM

[Absolute.]

Alright, I think I got it then.

for loops should always be < SIZE like:

// This function receives a column from the user
// checks the validity and then places the mark
void getCol(char boardArray[ROWS][COLS], char playerMark) 
{
	int column;
	
	// Asks for column number
	cout << "Choose a column: ";
	cin >> column;

	// Data check for column number
	while (column < 0 || column > 6)
	{
		cout << "Invalid Input!" << endl;
		cout << "Choose a column: " << endl;
		cin >> column;
	}

	int row = ROWS;				// Sets row to 6
	// Loops through all columns
	for (int col = 0; col < COLS; col++)
		// Checks column chosen
		if (col == column)
		{
			// Check to see if spot is empty
			while (boardArray[row][col] != '-')
			{
				// Data check for full column
				while (boardArray[row][col] != '-' && row == 0)			///	FIXXX
				{
					cout << "Column is Full!" << endl;
					cout << "Choose a column: ";
					cin >> column;
				}
				row--;
			}
			boardArray[row][col] = playerMark;
		}
}

// This function displays the current board
void showBoard(char boardArray[ROWS][COLS])
{
	cout << "         CONNECT FOUR       " << endl;
	cout << "  --------------------------- " << endl;
	cout << "   0   1   2   3   4   5   6" << endl;
	cout << "  --------------------------- " << endl;
	for (int rows = 0; rows < ROWS; rows++)
	{
		for (int cols = 0; cols < COLS; cols++)
			cout << " | " << boardArray[rows][cols];
		cout << " | \n";
	}
	cout << "  --------------------------- " << endl;
}

// This function clears the board and sets all locs to '-'
void clearBoard(char boardArray[ROWS][COLS])
{
	for (int rows = 0; rows < ROWS; rows++)
		for (int cols = 0; cols < COLS; cols++)
			boardArray[rows][cols] = '-';

}

*** Don't mind the FIX comment still trying to figure out how to test row == 0 and spot empty during the loop

[jimblumberg]

Quote

for loops should always be < SIZE

Yes. But:

 int row = ROWS;  
while (boardArray[row][col] != '-')

Is still a problem. ROWS is not less than ROWS.

Also:

while (column < 0 || column > 6)

You should probably change that 6 to the proper constant.

Jim

[Absolute.]

I believe I have "completed" my connect four game today after getting home from vacation. I am sure there are ways to improve the functions / logic, but I figured I would post my progress.

If anyone sees any huge flaws in logic or code, feel free to critique :]

/*
Conenct Four
By: Matthew Lepley
08/2011
*/

#include <iostream>
#include <string>

using namespace std;

// CONSTANTS
const int ROWS = 6;
const int COLS = 7;

// Prototypes
void changeMark(char& playerMark, string& playerPrompt);
void checkData(char boardArray[ROWS][COLS], char playerMark, int& column);
void placeMark(char boardArray[ROWS][COLS], char playerMark, int column);
void showBoard(char boardArray[ROWS][COLS]);
void clearBoard(char boardArray[ROWS][COLS]);
void checkWinner(char boardArray[ROWS][COLS], bool& gameOver);

int main() {
 
	// Variables
	char boardArray[ROWS][COLS];
	char playerMark;
	string playerPrompt = "Player 1 ";
	int column;
	bool test = false;
	bool gameOver = false;
	// Initialize board
	clearBoard(boardArray);
	showBoard(boardArray);

	while (gameOver == false) {
			
		// Asks for column number
		cout << playerPrompt << "Choose a column: ";
		cin >> column;
		
		changeMark(playerMark, playerPrompt);
		checkData(boardArray, playerMark, column);
		placeMark(boardArray, playerMark, column);
		showBoard(boardArray);
		checkWinner(boardArray, gameOver);
	}

	return 0;
}

// This function does error checking. Checks high and low for column input
// and to see if the current column is full or not.
void checkData(char boardArray[ROWS][COLS], char playerMark, int& column)
{
	// Data check for column number
	while (column < 0 || column > COLS - 1)
	{
		cout << "Invalid Input!" << endl;
		cout << "Choose a column: ";
		cin >> column;
	}
	
	// Data check for column full
	for (int col = 0; col < COLS; col++)
		if (col == column)
			while (boardArray[0][col] == 'X' || boardArray[0][col] == 'O')
			{
				cout << "Column is full!" << endl;
				cout << "Choose a column: ";
				cin >> column;
				col = column;			// Resets col to user input
			}
}

// This function places the players mark onto the board.
void placeMark(char boardArray[ROWS][COLS], char playerMark, int column)
{
	int row = ROWS - 1;				// Sets row to 5
	// Loops through all columns
	for (int col = 0; col < COLS; col++)
		// Checks column chosen
		if (col == column)
		{
			// Check to see if spot is empty
			while (boardArray[row][col] != '-')
				row--;
			boardArray[row][col] = playerMark;
		}
}

// This function switches playermarks for player 1 / 2
void changeMark(char& playerMark, string& playerPrompt)
{
	if (playerMark == 'X')
	{
		playerMark = 'O';
		playerPrompt = "Player 1 ";
	}
	else
	{
		playerMark = 'X';
		playerPrompt = "Player 2 ";
	}
}

// This function displays the current board
void showBoard(char boardArray[ROWS][COLS])
{
	cout << endl;
	cout << "         CONNECT FOUR       " << endl;
	cout << "  --------------------------- " << endl;
	cout << "   0   1   2   3   4   5   6" << endl;
	cout << "  --------------------------- " << endl;
	for (int rows = 0; rows < ROWS; rows++)
	{
		for (int cols = 0; cols < COLS; cols++)
			cout << " | " << boardArray[rows][cols];
		    cout << " | \n";
	}
	cout << "  --------------------------- " << endl;
}

// This function clears the board and sets all locs to '-'
void clearBoard(char boardArray[ROWS][COLS])
{
	for (int rows = 0; rows < ROWS; rows++)
		for (int cols = 0; cols < COLS; cols++)
			boardArray[rows][cols] = '-';
}

// This function tests for a winner
void checkWinner(char boardArray[ROWS][COLS], bool& gameOver)
{
	for (int rows = 0; rows < ROWS; rows++)
	{
		for (int cols = 0; cols < COLS; cols++)
			
			// Horizontal Player 1
			if(boardArray[rows][cols] == 'X' && boardArray[rows][cols + 1] == 'X' &&
			   boardArray[rows][cols + 2] == 'X' && boardArray[rows][cols + 3] == 'X')
			{
				cout << "Player 1 Wins!" << endl;
				gameOver = true;
			}
			
			// Horizontal Player 2
			else if (boardArray[rows][cols] == 'O' && boardArray[rows][cols + 1] == 'O' &&
				     boardArray[rows][cols + 2] == 'O' && boardArray[rows][cols + 3] == 'O')
			{
				cout << "Player 2 Wins!" << endl;
				gameOver = true;
			}

			// Vertical Player 1
			else if(boardArray[rows][cols] == 'X' && boardArray[rows +1][cols] == 'X' &&
			   boardArray[rows + 2][cols] == 'X' && boardArray[rows + 3][cols] == 'X')
			{
				cout << "Player 1 Wins!" << endl;
				gameOver = true;
			}
			
			// Vertical Player 2
			else if (boardArray[rows][cols] == 'O' && boardArray[rows + 1][cols] == 'O' &&
				     boardArray[rows + 2][cols] == 'O' && boardArray[rows + 3][cols] == 'O')
			{
				cout << "Player 2 Wins!" << endl;
				gameOver = true;
			}

			
			// Diagonal down Player 1
			else if(boardArray[rows][cols] == 'X' && boardArray[rows + 1][cols + 1] == 'X' &&
			   boardArray[rows + 2][cols + 2] == 'X' && boardArray[rows + 3][cols + 3] == 'X')
			{
				cout << "Player 1 Wins!" << endl;
				gameOver = true;
			}
			
			// Diagonal up Player 1
			else if(boardArray[rows][cols] == 'X' && boardArray[rows - 1][cols + 1] == 'X' &&
			   boardArray[rows - 2][cols + 2] == 'X' && boardArray[rows - 3][cols + 3] == 'X')
			{
				cout << "Player 1 Wins!" << endl;
				gameOver = true;
			}
			
			// Diagonal down Player 2
			else if (boardArray[rows][cols] == 'O' && boardArray[rows + 1][cols + 1] == 'O' &&
				     boardArray[rows + 2][cols + 2] == 'O' && boardArray[rows + 3][cols + 3] == 'O')
			{
				 cout << "Player 2 Wins!" << endl;
				 gameOver = true;
			}

			// Diagonal up Player 2
			else if(boardArray[rows][cols] == 'O' && boardArray[rows - 1][cols + 1] == 'O' &&
			   boardArray[rows - 2][cols + 2] == 'O' && boardArray[rows - 3][cols + 3] == 'O')
			{
				cout << "Player 2 Wins!" << endl;
				gameOver = true;
			}
	}
}

[baavgai]

You can typedef that puppy:

typedef char Board[ROWS][COLS];

// Prototypes
void getCol(Board, char playerMark);
void showBoard(Board);
void clearBoard(Board);

Better yet, classes are your friend:

class Board {
	static const int ROWS = 5, COLS = 6;
	enum CellType { CELL_EMPTY, CELL_PLAYER_1, CELL_PLAYER_2 };
	Board();
	// void getCol(char playerMark); // I'm unclear what this could mean...
	bool isValid(int columnNum) const;
	bool move(int columnNum);
	void show() const;
	void clear();
private:
	CellType data[ROWS][COLS];
	CellType currentPlayer;
};

Hope this helps.

[Absolute.]

Thanks for the tips, I haven't done much with typedefs and should probably learn them since I hear they save a lot of time (or is that templates?? or are they the same :X haha) I am confusing myself now.

As for the classes, that was the next step for me, thank you for showing me how to get started.