question on freeing memory

[degamer106]

If I have a dynimcally allocated array of objects and each of these objects contains a pointer to a string, when I use the 'delete' function to return memory, do I have to go through each array element and free each string individually or does the 'delete' take care of that for me in C++?

[John A]

I believe that you have to free each individual member of an object first (assuming of course, that the object contains data members that were dynamically allocated), before you can delete the actual object itself.

That's what the object's destructor is for; so use it wisely. If the actual object has a destructor that deallocates everything:

CObject::~CObject() {
    delete dynamicMember1;
    delete dynamicMember2;
// note that it's also a good practice to set pointers to 0
}

Once you've implement that, you can simply do this:

delete dynamicObject;

And dynamicObject's destructor will be called first.

[degamer106]

class State
{
public:
	State::State();
	State::~State();
	void initState(char * tempName, int tempYear, char * tempCap, int tempPopRank);
	void copyState(State & tempState);
	void printState();
private:
	char * name;	// state name
	int year;		// year of entry into the union
	char * cap;		// state capital
	int popRank;	// population ranking
};

class Database
{
public:
	Database(char * file = "states.txt");
	~Database(void);
	void deleteDatabase ();
	void fillDatabase ();
	void insertDatabase (int index, State & tempState);
	void printDatabase (bool flag = true);
private:
	State * list;		// array of states
	State tempState;
	int count;			// total count of states
	char * filename;	// input filename
};

Database::~Database(void)
{
//	cout << "Now in the Database destructor" << '\n';
	delete [] filename;
	delete [] list;
}

State::~State(void)
{
//	cout << "Now in the State destructor" << '\n';
//	delete [] name;
//	delete [] cap;
}

So in my program i've dynamically allocated an array of 3 state objects (to which list points to the beginning) and each of these objects contains pointers to dynamically allocated strings. In my whole program, I managed to get everything except the deletion to work properly. If I'm doing this correctly, then when I destroy the list, the State destructor should be called for each element in the array and thus free the strings right?

btw delete [] 'object name' is used to delete an array of objects right?

[John A]

So in my program i've dynamically allocated an array of 3 state objects (to which list points to the beginning) and each of these objects contains pointers to dynamically allocated strings. In my whole program, I managed to get everything except the deletion to work properly. If I'm doing this correctly, then when I destroy the list, the State destructor should be called for each element in the array and thus free the strings right?

Correct.

btw delete [] 'object name' is used to delete an array of objects right?

Yes.

Of course, it might work better if the delete statements in your destructor weren't commented out...

[degamer106]

yah but the problem is that when I uncomment those two statements out, my program crashes :eek:

I dunno if the rest of the program would help out...

using std::cout;
using std::cin;
using std::ios_base;

////////////////////////  State functions  //////////////////////////////
void State::initState(char * tempName, int tempYear, char * tempCap, int tempPopRank)
{
	char * stateName;
	char * capital;
   // if memory allocation is successful, store name
	if ( stateName = new char[strlen( tempName ) + 1 ] )
	{
		strcpy( stateName, tempName );
  		name = stateName;
	}
	else
	   cout << "Not enough memory!" << '\n';
   
	// store year
	year = tempYear;
   
	// if memory allocation is successful, store capital
	if ( capital = new char[strlen( tempCap ) + 1 ] )
	{
		strcpy( capital, tempCap );
		cap = capital;
	}
	else
		cout << "Not enough memory!" << '\n';
   
	// store population ranking
	popRank = tempPopRank;
}

//copyState - copies contents between two State structs
//input: pointer to source State struct
//       pointer to destination State struct
//return: nothing
void State::copyState (State & tempState)
{
	name = tempState.name;
	year = tempState.year;
	cap = tempState.cap;
	popRank = tempState.popRank;
}

//printState - displays State data
//input: pointer to State struct
//return: nothing
void State::printState ()
{
	cout.setf(ios_base::left);
	cout.width(17);
	cout << name;
	
	cout.setf(ios_base::left);
	cout.width(8);
	cout << year; 

	cout.setf(ios_base::left);
	cout.width(18);
	cout << cap ;

	cout.setf(ios_base::left);
	cout.width(2);
	cout << popRank << '\n';	

	return;
}

//////////////////////  Database functions  ////////////////////////////////
// Constructor
Database::Database(char * file)
{
//	cout << "Now in the Database constructor" << '\n';
	char * string;
	State * states;

	ifstream inputFile(file);
	// open input file and check for open success
	if ( inputFile )
	{
		// allocate memory for Database struct and check for success
		// allocate memory for filename field and initializes with filename
		if ( string = new char[strlen( file ) + 1] )
		{
			strcpy( string, file );
			filename = string;
		}
		else
		{
			cout << "Not enough memory!" << '\n';
			exit(0);
		}
		
		// initialize count field
		count = 5;
		
		// initialize list field by allocating memory for array of 50 states 
		// and check for success
		if ( states = new State[count] )
			list = states;
		else
		{
			cout << "Not enough memory!" << '\n';
			exit(0);
		}

		// close input file
		inputFile.close();
	}
	else
	{
		cout << "Error with file!" << '\n';
		exit(0);
	}
}

//fillDatabase - reads info from file and stores in array of State structs
//input: pointer to Database struct
//return: nothing
void Database::fillDatabase ()
{

	char name[80];	// temp variables
	int year;
	char cap[80];	// input data
	int popRank;
	int index = 0;	// index into array of State structs

	char array[100]; // temporary storage for file input

	// open file from filename field of db
	// don't need to check for file open success because initDatabase already checked
	ifstream inputFile( filename );
	
	// loop through all the states
	for ( int cnt = 0; cnt < count; cnt++ )
	{
	    // read and parse one line of file into 4 input data fields
		if ( !inputFile.eof() )
		{
			inputFile.getline( array, 100 );

			if ( sscanf( array, "%[^,], %d, %[^,], %d",	name, &year, cap, 
				&popRank ) == 4 )
			{
				// if tempState is created successfully from the 4 input data
				tempState.initState(name, year, cap, popRank);
				insertDatabase (index, tempState);
			}
			else
			{
				cout << "Error with file!" << '\n';
				return;
			}
			index++;
		}
	}
 
 	// close file
 	inputFile.close();
}

//printDatabase - print data of state array
//input: pointer to Database struct
//return: nothing
void Database::printDatabase (bool flag)
{
	cout << " State name      Year     Capital      Pop. rank" << '\n';
	cout << "------------------------------------------------" << '\n';

	if ( flag == true )
		for (int i = 0; i < count; i++)
			list[i].printState();
	else
		cout << "No data to display!" << '\n';
		
	return;
}

// main
	Database db;

	db.fillDatabase();
	db.printDatabase();

	return 0;

[John A]

Step through the program with your debugger. Watch any variables that you're going to need to delete later, and take note of the variables before the delete statement.

What I suspect is that you're somehow deleting memory that you did not allocate; or you're using delete[] when you didn't allocate an array (which you should then be using just delete ). delete doesn't usually fail unless you've done something wrong earlier.

[Salem]

1. If your destructors are always calling delete, then every single constructor you have MUST initialise the pointers to at least 0.

2. Your copystate function just makes copies of the pointers (you now have two pointers pointing at the same memory). Bad things happen when you try and delete both of them.