Dynamic array access violation

[JayGln]

I am working on a program that replicates the plinko game on the price is right. however my issue is that I am dynamically allocated a 2d array and having an exception reading violation. I can't see where I am allocating wrong or if it is deallocating wrong. If you happen to see anything else that I can optimize or if my logic is lacking please let me know.

 

template<class T>//add dynamic array parameter
ColLabeledTable<T>::ColLabeledTable(char startLbl, int lblWidth, int rows, int cols)
{
	startLabel = startLbl;
	labelWidth = lblWidth;
	row = rows;
	col = cols;
	table = new T *[rows];
	
	//set values to default
	for (int i = 0; i < rows; i++) {
		table[i] = new T[col];
		for (int j = 0; j < cols; j++) {
			table[i][j] = T();
		}
	}
}


template<class T>
ColLabeledTable<T>::ColLabeledTable(const ColLabeledTable &otherTable)
{
	//copy of non dynamic members
	//copy label
	 startLabel = otherTable.startLabel;
	//copy rows
	 col = otherTable.col;

	//copy columns
	 row = otherTable.row;

	//new pointer pointer
	//T **table;

	//allocate 1st array to row size
	table = new T *[row];

		
		for (int i = 0; i < row; i++) {
			//allocate 2d array
			table[i] = new T[col];
			for (int j = 0; j < col; j++) {
				//copy values in the 2d array
				table[i][j] = otherTable.table[i][j];
			}
		}

}


template<class T>
 ColLabeledTable<T> &ColLabeledTable<T>::operator=(const ColLabeledTable &otherTable)
{
	 //check for self assignment
	 if (this != &otherTable)
	 {

		 //deallocate old array
		 for (int i = 0; i < row; i++)
		 {
			 delete[] table[i];
		 }
		 delete[] table;
		 table = NULL;
		  //copy objects members

		 //non dynamic members
		 startLabel = otherTable.startLabel;
		 labelWidth = otherTable.labelWidth;
		 col = otherTable.col;
		 row = otherTable.row;

		 //allocate new array 
		 table = new T *[row];

		 for (int i = 0; i < row; i++) {
			 //allocate 2d array
			 table[i] = new T[col];
			 for (int j = 0; j < col; j++) {
				 //copy values in the 2d array
				 table[i][j] = otherTable.table[i][j];
			 }
		 }
		
	 }
	 return *this;

}

 //destructor
 template<class T>
 ColLabeledTable<T>::~ColLabeledTable()
 {
	 cout << "in destructor " << table[0][0];
	 //deallocate 2d array
	 for (int i = 0; i < col; i++)
		 delete[] table[i];
	 delete[] table;


	 //prevent dangling pointer
	 table = NULL;

	 //reset non dynamic members
	 startLabel = 'a';
	 row = 0;
	 col = 0;
 }

// Return number of rows in table.
template<class T>
int ColLabeledTable<T>::getNumRows() const
{
	return row;
}

// Return number of columns in table.
template<class T>
int ColLabeledTable<T>::getNumCols() const
{
	return col;
}

// Display table with column labels on standard output.
template<class T>
void ColLabeledTable<T>::display() const
{
	int i, j;

	// Display header with column labels.
	T colLabel = startLabel;
	for (j = 0; j < col; j++)
	{
		cout << "|" << setw(labelWidth) << colLabel;
		colLabel++;
	}
	cout << endl;

	// Nested loop to display values at all positions.
	T rowLabel = startLabel;
	for (i = 0; i < row; i++)
	{
		for (j = 0; j < col; j++)
		{
			// Display value at position.
			cout << setw(labelWidth + 1) << table[i][j];
		}

		cout << endl;  // each row on own line
	}
}


template<class T>
T ColLabeledTable<T>::getPos(int row, int col) const
{
	return table[row - 1][col - 1];
}

//rows and columns passed should be
// numbered starting with 1 (not zero) and be within
// size of table.
template<class T>
void ColLabeledTable<T>::setPos(int row, int col, const T &val)
{
	//set row col and char
	// Place value at specified position in table.
	table[row - 1][col - 1] = val;
}



const char EMPTY_CHAR = ' ';
const char PEG_CHAR = '.';
const char CHIP_CHAR = '*';



void generateBoard(ColLabeledTable<char> &board);


int advanceChip(const ColLabeledTable<char> &board, int &row, int &col);

// Test copying by invoking the copy constructor and
// assignment operator.
void testCopy(const ColLabeledTable<char> &board);

int main()
{
	// Allow user to enter size of board.
	// Although range checking would be useful, not required.

	int rows, cols;

	cout << "Enter # rows on board: ";
	cin >> rows;

	cout << "Enter # columns on board: ";
	cin >> cols;

	// Start "numbering" columns with letter below.
	const char COL_START = 'a';

	// Seed random number generator to get different
	// random sequence based on time.
	srand((unsigned int)time(NULL));

	// Create board representing game with starting
	// letter used to label columns and width of a
	// column label in # characters.
	ColLabeledTable<char> gameBoard(COL_START, 1, rows, cols);

	// Track score for game.
	int score = 0;

	cout << "\nLet's play Plincode!" << endl;

	do  // Play (potentially) multiple games.
	{
		// Generate new board with pegs and point values.
		generateBoard(gameBoard);

		// Display current score and initial board.
		cout << "\nScore: " << score << endl;
		gameBoard.display();

		// Test copying of game board.
		testCopy(gameBoard);

		// Store current position of chip.
		// Set to start at top.
		int chipRow = 0;
		int chipCol;
		char colLetter;

		cout << "\nEnter letter of starting column for chip (x=exit): ";
		cin >> colLetter;
		cin.ignore();  // eat newline

		// User wants to exit game?
		if (colLetter == 'x')
			break;

		// Translate from letter to 1-based column index.
		// Although validating character entered would be useful,
		// not required.
		chipCol = colLetter - COL_START + 1;

		// Let chip fall from top, being diverted if peg
		// is below it.
		advanceChip(gameBoard, chipRow, chipCol);

		do
		{
			// Place chip at its current position on board.
			gameBoard.setPos(chipRow, chipCol, CHIP_CHAR);

			// Display board configuration with chip.
			gameBoard.display();

			// Allow user to advance chip to next row.
			cout << "\nPress <Enter> for chip to fall to next row: ";
			string option;
			getline(cin, option);

			// Erase chip from its current position.
			gameBoard.setPos(chipRow, chipCol, EMPTY_CHAR);

			// Let chip fall one row, being diverted if peg
			// is below it.
			int points = advanceChip(gameBoard, chipRow, chipCol);

			// Increase score and start new game if reached bottom row.
			if (points != 0)
			{
				score += points;
				cout << "You earned " << points << " point(s)!" << endl;
				break;  // exit inner loop
			}

		} while (true);
	} while (true);

	cout << "\nWe hope you enjoyed Plincode!!" << endl;

	return 0;
}

// Definitions for functions that help set up/advance game.


void generateBoard(ColLabeledTable<char> &board)
{
	// FILL IN FUNCTION
	//use constants to set pegs and spaces
	//pegs are even, spaces are odd
	
	for (int i = 1; i < board.getNumRows(); i++) {
		for (int j = 1; j <= board.getNumCols(); j++) {
			if ((i + j) % 2 == 0)
			{
				 board.setPos(i, j, PEG_CHAR);
			}
			else {
				board.setPos(i, j, EMPTY_CHAR);
			}
		}
		
	}

	//set up last row in seperate for loop for the rand number generator
	for (int i = 0; i < board.getNumCols(); i++) 
	{
		board.setPos(board.getNumCols(), i + 1, ('a' + i) - 49);
		//for()
	}
	//	
	//for (int j = 1; j <= board.getNumCols(); j++)
	//{
	//	char randChar = 48 + rand() % board.getNumCols();
	//	board.setPos(board.getNumRows(), j, randChar);//(char)rand() % board.getNumCols() + 1);


	//}
	

}

// Advance chip's position to next row.  How it falls depends
// on whether a peg is present.  
int advanceChip(const ColLabeledTable<char> &board, int &row, int &col)
{
	//get the current position value
	//need to add 1 to row to get it on the first row
	char currPosition = board.getPos(row + 1, col);


	//test if it has empty char
	//kept separate for clarity
	if (currPosition == EMPTY_CHAR)
	{
		//advance to next row
		row++;
	}
	else {
		//returns score
		return currPosition;// +48;
	}

	//kept separate since if it is a peg there is no score to return
	//test if it has peg char 
	if (currPosition == PEG_CHAR)
	{
		//keeps it from falling off the left side
		if (currPosition == 1)
		{
			col++;
		}//keeps it from falling off the right side
		else if (currPosition == board.getNumCols()) 
		{
			col--;
		}
		else
		{
			//will make the move random
			if (rand() % 2 == 0)
			{
				col++;
			}else 
			{
				col--;
			}
		}
		//advance row
		row++;
	}


	
	
	return 0;  // no points earned
}

[ne555]

I'm not going to read ~400 lines of code.
post something that may be compiled and run through valgrind and a debugger.
also, provide whatever user input is needed.

> plinko game on the price is right
no idea what that is.

[JayGln]

Which file will be most helpful? My .h file or the definition .cpp file? I will obviously post my main.cpp file

[Ganado]

See these links:
http://sscce.org/
https://stackoverflow.com/help/mcve
https://en.wikipedia.org/wiki/Minimal_working_example

They key is not just dumping 400+ lines of code for us to look over. Someone might be willing to do that, but your chances of getting good help are minimal.

The key is seeing that the program as a whole probably isn't wrong; only a small part of it is wrong. Copy your project, and then start trimming out the parts of it that don't affect the particular problem. This will help narrow down the issue, and provide a smaller code base that other people would be more willing to read without eyes glazing over.

[jlb]

however my issue is that I am dynamically allocated a 2d array and having an exception reading violation. I can't see where I am allocating wrong or if it is deallocating wrong.

This exception is usually caused by trying to access an array out of bounds, or possibly trying to derefernce a nullptr.

Have you tried running the program with your debugger? Your debugger should be able to tell you exactly where it detects the problem and you should be able to view the affected variables at the time of the crash.

By the way why are you using nasty manual memory allocation instead of something like a std::vector?

Also is there a reason you've thrown in the complexity of templates into this program? Where is your class definition, with templates the definition and the implementation must be in the same compilation unit.

[JayGln]

ok so I hope this is good I will post the parts that I believe is the issue, I will tell you exactly which part the exception comes up. I have to use the template class, it is a project that my professor gave, which I know what logic (I believe) that I need to implement. So I have to use a 2d dynamic array, using a char to set the letters, pegs, spaces, and asterisk(players chip). I have tried using the visual studio debugger inserting breakpoints to go step by step through my main and cpp file. Is this the right information that you need?

this is my constructor to allocate the double pointer to a 2d dynamic array

template<class T>//add dynamic array parameter
ColLabeledTable<T>::ColLabeledTable(char startLbl, int lblWidth, int rows, int cols)
{
	startLabel = startLbl;
	labelWidth = lblWidth;
	row = rows;
	col = cols;
	table = new char *[rows];
	
	//int j = 0;
	//set values to default
	for (int i = 0; i < rows; i++) {
		table[i] = new char[cols];
		/*for ( ; j < cols; j++) {
			 table[0][j] = startLabel;
		}*/
	}
}

this is the part that it will through the exception. the user is throwing in an int starting from 1 in the main, but in my cpp it is zero indexed.

template<class T>
void ColLabeledTable<T>::setPos(int row, int col, const T &val)
{
	//set row col and char
	// Place value at specified position in table.
	table[row - 1][col - 1] = val;
}

[jonnin]

well, if that is where it chokes, try this:

template<class T>
void ColLabeledTable<T>::setPos(int row, int col, const T &val)
{
	//set row col and char
	// Place value at specified position in table.
        assert(row>0 && col > 0);
        assert(row<somemaxvalueforrows && col < somemaxvalueforcols);
	table[row - 1][col - 1] = val;
}

[jlb]

I will tell you exactly which part the exception comes up.

If you run the program with your debugger without any breakpoints set it should be able to tell you exactly where it encounters the problem.

So where exactly is the program crashing?

Whenever you do addition and subtraction to an array index you have a potential source of out of bounds array access, you should consider testing that the values are within the range of the array. And remember that arrays in C start at zero and stop at size - 1.

Is this the right information that you need?

No, please read all of the links provided by @Ganado in his post above and provide the smallest possible complete program that illustrates the problem. What you have posted so far is neither the smallest possible, or even a complete program.

[jonnin]

what I always did when dealing with index-from-1 was just allocate an extra space.
eg if you need a 10x10 matrix, do this:
double mat[11][11]; and just ignore the 0 locations. It wastes a little space to save a lot of hassle, and its great so long as the matrices are not gigantic.

if using pointers instead of arrays, you can actually cheat :)

int ** ip = new int[10];
ip[0] = new int[10];
ip[0]--; //find some way to do pointer math to subtract 1 from the pointer, so you can index from 1 again... remember that to delete you have to move it back to the 0 location first... but it should work with a little messing with it to get the syntax happy. Just don't access it directly, the 0 location is out of bounds, so its a little hands-on and risky if you are not very precise.

[ne555]

> just allocate an extra space.
also good advice if you need to check the neighbours of your cell so there's no special case for the borders.

[JayGln]

Ok that is great advice I will try allocating an extra space I know I will get penalized for it but I can work on it on my own time later to learn how to do it better. Now please forgive me, I was just trying to learn visuals memory detection tool with the snapshot. while I was using it I saw when I created the new memory and when I didn't. so I found that in my overloaded assignment operator it was only creating one instance of my array and not allocating any memory to the second one inside. I'm going to try and make as much sense of it as I can and explain then show my assignment operator....I took a few snapshots and to start my char*[] has a count of 1, my char[] has a count of 6. Through the various snapshot I took from beginning(no memory allocated) my constructor, copy constructor, always stayed with that ratio 1:6 , 2:12. except my assignment operator that now shows char*[] as 2 count and my char[] as 6. So I know it is not allocating for the other 6, just syntactically I don't understand. here is my constructor, copy, assign, and destruct

template<class T>//add dynamic array parameter
ColLabeledTable<T>::ColLabeledTable(char startLbl, int lblWidth, int rows, int cols)
{
	startLabel = startLbl;
	labelWidth = lblWidth;
	row = rows;
	col = cols;
	table = new T *[rows];
	
	//int j = 0;
	//set values to default
	for (int i = 0; i < rows; i++) {
		table[i] = new T[cols];
		/*for ( ; j < cols; j++) {
			 table[0][j] = startLabel;
		}*/
	}
}

//copy constructor
template<class T>
ColLabeledTable<T>::ColLabeledTable(const ColLabeledTable &otherTable)
{
	//copy of non dynamic members
	//copy label
	cout << "this is copy constructor " << endl;

	 startLabel = otherTable.startLabel;
	 labelWidth = otherTable.labelWidth;
	//copy rows
	 col = otherTable.col;

	//copy columns
	 row = otherTable.row;

	//allocate 1st array to row size
	table = new T *[otherTable.row];

		
		for (int i = 0; i < row; i++) {
			//allocate 2d array
			table[i] = new T[otherTable.col];
			for (int j = 0; j < col; j++) {
				//copy values in the 2d array
				table[i][j] = otherTable.table[i][j];
			}
		}

}

//overloaded assignment operator
template<class T>
 ColLabeledTable<T> &ColLabeledTable<T>::operator=(const ColLabeledTable &otherTable)
{
	 //check for self assignment
	 if (this != &otherTable)
	 {

		 //deallocate old array
		 for (int i = 0; i < row; i++)
		 {
			 delete[] table[i];
		 }
		 delete[] table;
		 table = NULL;
		  //copy objects members

		 //non dynamic members
		 startLabel = otherTable.startLabel;
		 labelWidth = otherTable.labelWidth;
		 col = otherTable.col;
		 row = otherTable.row;

		 //allocate new array
		 
		 table = new T *[otherTable.row];


		 for (int i = 0; i < row; i++) {
			 //allocate 2d array
			 table[i] = new T[otherTable.col];
			 for (int j = 0; j < col; j++) {
				 //copy values in the 2d array
				 table[i][j] = otherTable.table[i][j];
			 }
		 }
		
	 }
	 return *this;

}

 //destructor
 template<class T>
 ColLabeledTable<T>::~ColLabeledTable()
 {
	 cout << "in destructor " << table[0][0];
	 //deallocate 2d array
	 for (int i = 0; i < col; i++)
		 delete[] table[i];
	 delete[] table;


	 //prevent dangling pointer
	 table = NULL;

	 //reset non dynamic members
	 startLabel = 'a';
	 row = 0;
	 col = 0;
	 cout << "after destryc " << table[0][0];
 }

Thank you everyone, I appreciate everything I really want to get better, I know I have a long road ahead to be professionally ready

[ne555]

> cout << "after destryc " << table[0][0];
great way of blowing off your foot.
`table' is a null pointer at that point, don't dereference null pointers.

[Ganado]

There's also no point in resetting values after something is destructed, because nothing should be able to access that object after it's been destructed anyway. It's either gone out of scope, or been deleted by the user.