Destructor Problem causing an exit exception.

[CryHavocMike]

I've been searching this problem for a couple days now and nothing I try seems to work. I'm getting it when my program ends, otherwise the program runs just fine so I "think" it has something to do with the Destructor. But I've tried looping through arrPtr and deleting all the elements first and it doesn't fix it. I'm using Visual Studio.

I ran across this thread http://www.cplusplus.com/forum/general/19273/ and I think its saying what I need but I just don't know enough to understand what all is going on in it.

So I wrote the following 'Test' code that is basically the operations my code is doing so it wont make any real sense as to what it "does".... But it throws the exact same error as main code so but is much smaller to present for help.

I was hoping/thinking the Destructor would "unallocate" the array once the program ended as I can't see where else to do it.

// Test Code.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"

#include <iostream>
#include <string>

using namespace std;

class Works
{
public:
	Works();
	~Works();

private:
	static const int ROWS = 3;     // holds # of array rows.
	static const int COLS = 3;     // Sets # of array columns.
	char(*arrPtr)[COLS] = new char[ROWS][COLS];
	int newR = 0;
	int newC = 2;

public:
	void getArr(char(&)[ROWS][COLS]);
	void sendArrMod(int, int);

};

Works::Works()
{
	
}

Works::~Works()
{
	for (int i = 0; i<ROWS; i++)   // deallocation
		delete[] arrPtr[i];
	delete[] arrPtr;
}

void Works::getArr(char(&temp)[ROWS][COLS])
{
	cout << "temp array: " << *temp << endl;

	arrPtr = temp;

	sendArrMod(newR, newC);
}

void Works::sendArrMod(int newr, int newc)
{
	arrPtr[newr][newc] = 'N';

}

// Global constants
const int ROWS = 3;     // holds # of array rows.
const int COLS = 3;     // Sets # of array columns.

// Define and initialize Global variable tic-tac-toe array.
char tttoeArray[ROWS][COLS] = { { '*', '*', 'c' },
{ 'c', 'c', '*' },
{ '*', 'X', 'X' } };

int main()
{
	Works Temp;

	cout << "Orig. array: " << *tttoeArray << endl;

	Temp.getArr(tttoeArray);

	cout << "New array: " << *tttoeArray << endl;

	system("pause");

	return 0;

}

[Thomas1965]

How do you manage to run this program? I get some errors:

main.cpp(15): error C2864: 'Works::arrPtr' : only static const integral data members can be initialized within a class
main.cpp(16): error C2864: 'Works::newR' : only static const integral data members can be initialized within a class
main.cpp(17): error C2864: 'Works::newC' : only static const integral data members can be initialized within a class
main.cpp(33): warning C4154: deletion of an array expression; conversion to pointer supplied

[TheIdeasMan]

@Thomas1965

That isn't a problem with c++11, initialising in a class is the same as if a member initialiser list had been used for those variables.

@CryHavocMike

It looks like you are attempting to delete your array twice - one call to delete[] should be sufficient, there is no need to try and delete each element.

Ideally, one shouldn't be using new and delete anyway, instead consider using smart pointers. These get around the problem of an exception happening before the destructor is reached. Google unique_ptr

Good Luck !!

[ne555]

Don't see any need to use dynamic allocation.
If your real code does need to use it, consider a std::vector

> arrPtr = temp;
leak, and invalid delete on the destructor.
you need `std::copy()' or `memcpy()'


> I'm getting it when my program ends, otherwise the program runs just fine so
> I "think" it has something to do with the Destructor.
you've got a memory corruption problem.
your destructor may be correct (in this case it is not), but the meddling that do the other functions may be giving it invalid pointers to work on.

may use tools such as valgrind to detect invalid frees, but you'll have to track the code to figure out why does it happens.
Or instead you may follow the rule of zero and don't have to worry about anything.
http://en.cppreference.com/w/cpp/language/rule_of_three


> there is no need to try and delete each element.
clang even gives you an error (gcc, a cryptic warning)

foo.cpp:38:3: error: cannot delete expression of type 'char [3]'
                delete[] arrPtr[i];
                ^        ~~~~~~~~

[Thomas1965]

@TheIdeasMan

@Thomas1965

That isn't a problem with c++11, initialising in a class is the same as if a member initialiser list had been used for those variables.

char(*arrPtr)[COLS] = new char[ROWS][COLS];

Is this really valid synthax ?

[48T7M4Gy]

char **twoD = new char*[sizeY];
for(int i = 0; i < sizeY; ++i) {
twoD [i] = new char[sizeX];
}

and then clean up would be:

for(int i = 0; i < sizeY; ++i) {
delete [] twoD [i];
}
delete [] twoD ;

http://stackoverflow.com/questions/936687/how-do-i-declare-a-2d-array-in-c-using-new

[JLBorges]

> char(*arrPtr)[COLS] = new char[ROWS][COLS];
> Is this really valid syntax ?

Yes. COLS is a constant known at compile-time.

Though this would be a lot better. ROWS is also a constant known at compile-time.
char arr[ROWS][COLS] = {} ;

[48T7M4Gy]

char **twoD = new char*[sizeY]; etc would only apply if twoD is allocated dynamically (which is not necessary as ne555 pointed out) and a static array is destroyed automatically once it is out of scope.

[Thomas1965]

@kemort,

thanks. I will check out this new synthax, even when VS 2010 doesn't support it.

[48T7M4Gy]

@Thomas1965
I'd be surprised if it doesn't work with VS2010. It's a common approach for 2D (and beyond) matrix classes which I first used way before that. I like the stackoverflow rendition for its clarity in my overcoming the multiple *'s etc :)

[Thomas1965]

@kemort

the code you posted compiles.

char **twoD = new char*[sizeY];
for(int i = 0; i < sizeY; ++i) 
{
  twoD [i] = new char[sizeX];
}

It seems VS 2010 doesn't like this synthax:

char(*arrPtr)[COLS] = new char[ROWS][COLS];

[CryHavocMike]

Hello Kemort,

I'm very familiar with that stackoverflow link and your approach. but the reason I need the new 2D array is to be able to dump information into it from outside the class, so that other class functions can have access to that information. The approach falls apart when I try to assign the information coming in, to the new array. It becomes a rabbit chase of incompatible types I can't seem to solve.

private:
	static const int ROWS = 3;     // holds # of array rows.
	static const int COLS = 3;     // Sets # of array columns.
	//char(*arrPtr)[COLS] = new char[ROWS][COLS];
	char **arrPtr = new char*[COLS];  <=== 2) into here?
	int newR = 0;
	int newC = 2;

void Works::getArr(char(&temp)[ROWS][COLS]) <=== 1) How do I get this argument
{
	cout << "temp array: " << *temp << endl;

	arrPtr = temp; <=== 3) Because this generates an error that wont compile.
	
	for (int i = 0; i < COLS; ++i) {
		arrPtr[i] = new char[ROWS];
	}

	sendArrMod(newR, newC);
}

@ Thomas1965
I'm using Visual Studio Community 2013 and that statement works there.

[48T7M4Gy]

Hi Thomas1965 and CryHavocMike

Please don't interpret my introducing the ** orthodoxy as solving any problem with the OP. It was purely a response to a comment that ne555 made, a contrafactual if you like.

I haven't put any thought into it but the single * way seems strange to me and the static keyword rings warning bells. As far as class methods are concerned, nothing special is needed to access the ** array.

The array can be created at any time in the life of the object. If it has to be changed in size during the life of the object then this is not an impossibility. More tedious than anything and probably a good case for the dreaded vector vs array solution favouring vectors in this case.

My apologies if the ** way doesn't suit you for whatever reason and maybe you should just use the [][] approach as indicated originally and by Borges. You'll still have the resize() problem though if that's the issue.

[CryHavocMike]

Oh... well okay.

But a 'heavy' vector or static array generates it own problems as well.

The "static" keywords are there because VS wouldn't shut up about it. The array in main uses constant ints so when trying to make its reference equal to the new dynamic array VS would throw all kinds of warnings and not compile. The "static" shut it up.

If I pseudo code it;

My original approach is to;

- Accept a reference to a small 3x3 2D array from outside the class.
- Assign the reference to a new dynamic array inside the class other class functions can peek at.
- Once the class functions determine which element in the array to change,
- Send a row and column integer to a class function which makes the change to the reference==> which changes the element in the original array.

This means only one array is actually changed which I thought would be a good approach. However when the class ends all hell breaks loose trying to unallocate the new dynamic array. Sure, a dynamic array for a simple array is overkill but it worked beautifully..... right up until the class ends.

If I make a static array or vector in the class as ne555 suggests then what? I'm not making it simply to be making it. So now I would have an array outside the class and a vector inside the class how does that help me?

[JLBorges]

#include <iostream>

class Works
{
    public:
        static constexpr int NROWS = 3;
        static constexpr int NCOLS = 3;
        using board_type = char[NROWS][NCOLS] ;

    private:
        board_type board = { { '1', '2', '3' }, { '4', '5', '6' }, { '7', '8', '9' } } ;

    public:

        void modify_board( int row, int col, char c = '*' )
        {
            if( row < NROWS && col < NCOLS ) board[row][col] = c ;
        }

        void print_board( std::ostream& stm = std::cout ) const
        {
            for( const auto& row : board )
            {
                for( char c : row ) stm << c << ' ' ;
                std::cout << '\n' ;
            }
            std::cout << "------\n" ;
        }
};

int main()
{
    Works w ;
    w.print_board() ;

    w.modify_board( 2, 0 ) ;
    w.modify_board( 1, 1, '+' ) ;
    w.print_board() ;
}

http://rextester.com/IHMMRQ94663
http://coliru.stacked-crooked.com/a/db61ce766903d8b2

[ne555]

When you do arrPtr = temp; you are simply pointing to that array.
Now, your object does not own that array and in that case it must not delete it.

Also, you don't need to allocate anything.

char(*arrPtr)[COLS]; //no dynamic allocation
Works(char *temp[COLS]): arrPtr(temp) {} //link it on the constructor

//not managing anything
Works(const Works &) = default;
Works& operator=(const Works&) = default;

you must make sure that the array outlives the `Works' object or you may do an invalid access

[48T7M4Gy]

Maybe I'm missing something, but if the source array is always 3x3 and the class array is always 3x3 then all you do is piecewise copy the elements into the class array[][] either via the constructor or a setter function.

[48T7M4Gy]

#include <iostream>
#include <string>

using std::cout;
using std::endl;

class Works
{
public:
	Works();
	~Works();

private:
	const int ROWS = 3;
	int COLS = 3;
	char matrix[3][3];

public:
	void setArray(char[][3]);
	void printArray();
};

Works::Works()
{
	//DEFAULT
	for (int r = 0; r < ROWS; r++)
	{
		for (int c = 0; c < COLS; c++)
			matrix[r][c] = '?';
	}
};

Works::~Works() {};

void Works::setArray(char anArray[][3])
{
	for (int r = 0; r < ROWS; r++)
	{
		for (int c = 0; c < COLS; c++)
			matrix[r][c] = anArray[r][c];
	}
}

void Works::printArray()
{
	for (int r = 0; r < ROWS; r++)
	{
		for (int c = 0; c < COLS; c++)
			cout << matrix[r][c] << ' ';
		cout << endl;
	}
}

/*===================================================*/
void printArray(char[][3]);

int main()
{
	const int ROWS = 3;
	const int COLS = 3;

	char tttoeArray[3][3] =
	{
		{ '*', '*', 'c', },
		{ 'c', 'c', '*' },
		{ '*', 'X', 'X' }
	};

	printArray(tttoeArray);

	Works temp;
	temp.printArray();

	temp.setArray(tttoeArray);
	temp.printArray();

        return 0;
}

void printArray(char anArray[][3])
{
	for (int r = 0; r < 3; r++)
	{
		for (int c = 0; c < 3; c++)
			cout << anArray[r][c] << ' ';
		cout << endl;
	}
}

Yet another permutation if you don't like 'static'.