Destructor - Execution problem

I'm writing a program that has execution problems when using a destructor. I have provided a default constructor, parameterized constructor, copy constructor, and overloaded assignment operator. The destructor is used because of variables on the heap memory.
I've looked over everything a number of times for syntactical errors, but this is something i'm not familiar with, or am overlooking. Could someone more familiar with C++ point out to me what is readily apparent to them?
Thanks,
anthony

Code:

//footballgame.h interface file
#if !defined(FOOTBALLGAME_H)	
#define FOOTBALLGAME_H

class FootballGame
{
public:

FootballGame(void); //default constructor declaration
//parameterized constructor declaration
FootballGame(char* szName, char* szCity, char* szQB, char* szMascot);
FootBallGame(const FootballGame&);//copy constructor 

~FootballGame(void);//destructor declaration
FootballGame& operator = (const FootballGame&);
friend FootballGame operator ++ (const FootballGame&);
void setTeamName(char* szName);
char* getTeamName() const;
void setCity(char* szCity);
char* getCity() const;
void setQB(char* szQB);
char* getQB() const;
void setMascot(char* szMascot);
char* getMascot() const;
static int getScore();
private:
static int iScore;
char* szTeamName;
char* szCityName;
char* szQBName;
char* szMascotName;
};//end class FootballGame

#endif
     
///////////////////////////////////////////
//footballgame.cpp implementation file

#include "footballgame.h"
#include <cstring>
#include <ostream.h>

int FootballGame::iScore = 0;

FootballGame::FootballGame(void)
{//default constructor definition
	szTeamName   = new char[25];
	strcpy(szTeamName,"");
	szCityName   = new char[25];
	strcpy(szCityName,"");
	szQBName     = new char[25];
	strcpy(szQBName,"");
	szMascotName = new char[25];
	strcpy(szMascotName,"");
}

FootballGame::FootballGame(char* szName, char* szCity, 
char* szQB, char* szMascot)
{//parameterized constructor definition
	szTeamName   = new char[25];
	strcpy(szTeamName,szName);
	szCityName   = new char[25];
	strcpy(szCityName,szCity);
	szQBName     = new char[25];
	strcpy(szQBName,szQB);
	szMascotName = new char[25];
	strcpy(szMascotName,szMascot);
}

FootballGame::FootBallGame(const FootballGame& sourceTeam)
{//copy constructor definition
	szTeamName   = new char[25];
	strcpy(szTeamName,sourceTeam.szTeamName);
	szCityName   = new char[25];
	strcpy(szCityName,sourceTeam.szCityName);
	szQBName     = new char[25];
	strcpy(szQBName,sourceTeam.szQBName);
	szMascotName = new char[25];
	strcpy(szMascotName,sourceTeam.szMascotName);
	iScore = sourceTeam.iScore;
}

FootballGame::~FootballGame(void)
{//destructor definition
	delete[] szTeamName;
	delete[] szCityName;
	delete[] szQBName;
	delete[] szMascotName;
}

FootballGame& FootballGame:: operator = (const FootballGame& operand)
{//defines overloaded assignment operator
	if(this == &operand)
		return *this;
	else
	{
	szTeamName   = new char[25];
	strcpy(szTeamName,operand.szTeamName);
	szCityName   = new char[25];
	strcpy(szCityName,operand.szCityName);
	szQBName     = new char[25];
	strcpy(szQBName,operand.szQBName);
	szMascotName = new char[25];
	strcpy(szMascotName,operand.szMascotName);
	iScore = operand.iScore;
	return *this;
	}
}

FootballGame operator ++ (const FootballGame& operand)
{//defines overloaded increment operator
	FootballGame game;
	game.szCityName = new char[25];
	strcpy(game.szCityName,operand.szCityName);
	game.szMascotName = new char[25];
	strcpy(game.szMascotName, operand.szMascotName);
	game.szQBName = new char[25];
	strcpy(game.szQBName, operand.szQBName);
	game.szTeamName = new char[25];
	strcpy(game.szTeamName, operand.szTeamName);
	game.iScore = operand.iScore + 6;
	return game;
}

void FootballGame::setCity(char* szCity)
{
	strcpy(szCityName,szCity);
}

char* FootballGame::getCity() const
{
	return szCityName;
}

void FootballGame::setMascot(char* szMascot)
{
	strcpy(szMascotName,szMascot);
}

char* FootballGame::getMascot() const
{
	return szMascotName;
}

void FootballGame::setQB(char* szQB)
{
	strcpy(szQBName,szQB);
}

char* FootballGame::getQB() const
{
	return szQBName;
}

void FootballGame::setTeamName(char* szName)
{
	strcpy(szTeamName,szName);
}

char* FootballGame::getTeamName() const
{
	return szTeamName;
}

int FootballGame::getScore()
{
	return iScore;
}

//////////////////////////////////////
//user.cpp client source file

#include <iostream>
#include <cstring>
#include "footballgame.h"
using namespace std;

int main(void)
{
	FootballGame game("Green Bay Packers", "Green Bay", 
		"Favre", "Cheese Heads");
	cout << "Team: " << game.getTeamName() << endl;
	cout << "Mascot: " << game.getMascot() << endl;
	cout << "Quarterback: " << game.getQB() << endl;
	cout << "City: " << game.getCity() << endl;
	game = ++game;
	game = ++game;
	
	cout << game.getScore() << endl;
	game = ++game;
	cout << game.getScore() << endl;
	game = ++game;
	cout << game.getScore() << endl;
	
                return EXIT_SUCCESS;
}//end main

Please to use code tags.

[.code]your_code[./code]

Remove the periods.

sorry, wrong operation!

I try to edit some error , now it's pass compilation.

Code:

#include <iostream>    //not ostream!
#include <string>
using namespace std;

class FootballGame
{
public:

FootballGame(void); //default constructor declaration
//parameterized constructor declaration
FootballGame(char* szName, char* szCity, char* szQB, char* szMascot);
int FootBallGame(const FootballGame& );//copy constructor  //add int type define

~FootballGame(void);//destructor declaration
FootballGame& operator = (const FootballGame&);
friend FootballGame operator ++ (const FootballGame&);
void setTeamName(char* szName);
char* getTeamName() const;
void setCity(char* szCity);
char* getCity() const;
void setQB(char* szQB);
char* getQB() const;
void setMascot(char* szMascot);
char* getMascot() const;
static int getScore();
private:
static int iScore;
char* szTeamName;
char* szCityName;
char* szQBName;
char* szMascotName;
};//end class FootballGame


int FootballGame::iScore = 0;

FootballGame::FootballGame(void)
{//default constructor definition
szTeamName = new char[25];
strcpy(szTeamName,"");
szCityName = new char[25];
strcpy(szCityName,"");
szQBName = new char[25];
strcpy(szQBName,"");
szMascotName = new char[25];
strcpy(szMascotName,"");
}

FootballGame::FootballGame(char* szName, char* szCity, 
char* szQB, char* szMascot)
{//parameterized constructor definition
szTeamName = new char[25];
strcpy(szTeamName,szName);
szCityName = new char[25];
strcpy(szCityName,szCity);
szQBName = new char[25];
strcpy(szQBName,szQB);
szMascotName = new char[25];
strcpy(szMascotName,szMascot);
}

int FootballGame::FootBallGame(const FootballGame& sourceTeam)
{//copy constructor definition
szTeamName = new char[25];
strcpy(szTeamName,sourceTeam.szTeamName);
szCityName = new char[25];
strcpy(szCityName,sourceTeam.szCityName);
szQBName = new char[25];
strcpy(szQBName,sourceTeam.szQBName);
szMascotName = new char[25];
strcpy(szMascotName,sourceTeam.szMascotName);
iScore = sourceTeam.iScore;
}

FootballGame::~FootballGame(void)
{//destructor definition
delete[] szTeamName;
delete[] szCityName;
delete[] szQBName;
delete[] szMascotName;
}

FootballGame& FootballGame:: operator = (const FootballGame& operand)
{//defines overloaded assignment operator
if(this == &operand)
return *this;
else
{
szTeamName = new char[25];
strcpy(szTeamName,operand.szTeamName);
szCityName = new char[25];
strcpy(szCityName,operand.szCityName);
szQBName = new char[25];
strcpy(szQBName,operand.szQBName);
szMascotName = new char[25];
strcpy(szMascotName,operand.szMascotName);
iScore = operand.iScore;
return *this;
}
}

FootballGame operator ++ (const FootballGame& operand)
{//defines overloaded increment operator
FootballGame game;
game.szCityName = new char[25];
strcpy(game.szCityName,operand.szCityName);
game.szMascotName = new char[25];
strcpy(game.szMascotName, operand.szMascotName);
game.szQBName = new char[25];
strcpy(game.szQBName, operand.szQBName);
game.szTeamName = new char[25];
strcpy(game.szTeamName, operand.szTeamName);
game.iScore = operand.iScore + 6;
return game;
}

void FootballGame::setCity(char* szCity)
{
strcpy(szCityName,szCity);
}

char* FootballGame::getCity() const
{
return szCityName;
}

void FootballGame::setMascot(char* szMascot)
{
strcpy(szMascotName,szMascot);
}

char* FootballGame::getMascot() const
{
return szMascotName;
}

void FootballGame::setQB(char* szQB)
{
strcpy(szQBName,szQB);
}

char* FootballGame::getQB() const
{
return szQBName;
}

void FootballGame::setTeamName(char* szName)
{
strcpy(szTeamName,szName);
}

char* FootballGame::getTeamName() const
{
return szTeamName;
}

int FootballGame::getScore()
{
return iScore;
}


int main(void)
{
FootballGame game("Green Bay Packers", "Green Bay", 
"Favre", "Cheese Heads");
cout << "Team: " << game.getTeamName() << endl;
cout << "Mascot: " << game.getMascot() << endl;
cout << "Quarterback: " << game.getQB() << endl;
cout << "City: " << game.getCity() << endl;
game = ++game;
game = ++game;

cout << game.getScore() << endl;
game = ++game;
cout << game.getScore() << endl;
game = ++game;
cout << game.getScore() << endl;

system("pause");                   //add pause command
return EXIT_SUCCESS;
}//end main

Thw changes i see are the int keyword added before the copy constructor declaration and definition in the interface and implementation files, and the system("pause"); command statement before the return EXIT_SUCCESS; statement of the int main() function in the client class (user source file). A change is also made to the #include directive of the implementation file that includes the ostream class - to iostream. The ostream is all that is needed for the implementation file because you may wish to use a cout statement - though not required - and there is no user input in the implementation source file.

YES, the program will pass compilation - compilation was NEVER a question - but during run-time execution, this is where the problem occurs. The syntax is acceptable, the logic is flawed. Any additions you have made, though welcome, do not resolve the problems previously cited with run-time execution.

When writing the operator=, you must free the memory that was originally stored in the object, since operator= modifies an already constructed object. Move the code in the destructor to a cleanup function, then call that function inside both the destructor and the operator= before you assign the new data.

In addition, your operator++ makes no sense. What are you trying to do? Usually operator++ is called on a single value to increment it, but that is not exactly how you are using it.

You have to define what type of problem you are having and why you think it is with the destructor.

I have several concerns as I review your code.

I think there are several problems with the ++ operator. The first is that the ++ operator is usually a unary operator. It changes values within the object on which it is called. It doesn't need any parameters and it doesn't need a return value. In your version you have a parameter passed in, meaning it isn't unary any longer. This might not be a problem from the code's point of view, but logically it's like overloading the + operator to do -. The ++ operator delaration line doesn't allow for a return value, yet you have one listed in the code. That's illogical, too. Also, the code declares new memory for the all the variables, but doesn't delete the memory that was originally assigned to those variables. That memory is therefore lost and can't be released when the destructor is called. That creates a memory leak and makes it look like an error in the destructor, but it's actually in the ++ operator.

Since all the char * member variables are declared privately. This means when you pass a reference to the class as a parameter to a method/operator of the current object, like with the copy constructor or the assignment operator, you need to access the member variables of the passed object with the public accessor methods you have declared, since the passed object isn't the "this" object.


//copy constructor definition
szTeamName = new char[25];
strcpy(szTeamName,sourceTeam.getTeamName());

etc.

Maybe that will help you some. Maybe not. I haven't compiled the original code or rewritten the code with my corrections to see if they work, but I think I am correct.

Edit: well I see that jlou had posted a response while I was composing mine. It's reassuring to see that he/she had concerns similar to mine.

It's good to know the things that have been shared. The book i am using has no other references for overloaded unary operators, therefore the code that you see - there also aren't any more detailed or satisfactory explanations and examples within the text: Author - Don Gosselin, Book - Microsoft Visual C++ 6.0.
There's not much i or anyone who invests 50 - 100 dollars for a text appreciates than an expert's ignorance or omissions when authoring a text. like most people, i take serious the endeavors i pursue, and the book isn't worth the paper it's printed on.
For example, a quote from the text:
"When you overload a unary operator, the overloaded function definition includes only a single parameter in the parameter list since unary operators use only single operands. The syntax for an overloaded unary operator function is as follows:
class operator unary_operator (const class& operand) { }

The only definition example is as follows:
Stocks operator ++ (const Stocks& operand)
{
Stocks roundLot;
roundLot.iNumShares = operand.iNumShares + 100;
return roundLot;
}"

And now i learn here that i don't need a parameter. the obvious concerns concerning memory management (particularly with variables on the heap and cleaning them up - releasing from memory) are NOT being adequately discussed in the text, and this particular chapter (5) is supposed to be dedicated to memory management. My belief, if it can be crashed, the author of a text didn't do what he was paid for and probably should not have been recommended to produce a text anyway. wow! guess i'll try googles...

>>wow! guess i'll try googles...
You'll sometimes run into the same problems with google (and with many of the tutorials/books you find), but google does has the advantage of being free

i've found more informative and ready repsonses as well - greater depth of explanation and reference points.

This is appropriate for what i was originally trying to do:

//declarations of overloaded operators in .h file
void operator ++();
void operator ++(int);

//definitions of overloaded increment operators in .cpp implementation file
void FootballGame::operator ++()
{//defines prefix overloaded unary increment operator

iScore += 6;

//touchdown scored - no extra point
}

void FootballGame::operator ++ (int iScores)
{//defines postfix overloaded unary increment operator

iScore += 7;

//touchdown with successful PAT
}

//call to overloaded functions from user/client .cpp
Football game;
++game;//uses prefix operator to increment score
cout << game.getScore() << endl;
game++;//uses postfix operator to increment score
cout << game.getScore() << endl;

what i would still like to see is the resolution of the original declaration and implementation of the overloaded prefix increment operator where referencing a const class object, as seemed to be implied by the author of the text i am using. there were assert failures during execution that should have been approached and addressed in the text to avoid such roadblocks...

All right here's the example code for overloading ++ operator, both prefix and postfix.

Code:

Class Counter
{
   public:
	  const Counter& operator++();
	  const Counter& operator++(int theFlag);
	
   private:
	  int itsVal;
};
 
//prefix
const Counter& Counter::operator++()
{
  ++itsVal;
  return *this;
}
 
//postfix
const Counter& Counter::operator++(int theFlag)
{
   Counter temp(*this);
   ++itsVal;
   return temp;
}

The author states:

How is it possible to differentiate between the prefix and postfix version. By convention, an int variable is supplied as the parameter in the postfix version, but it is just a signal that is ignored.

Therefore I suppose you could pass a reference as a parameter. But you never use the parameter in the code for the function body.

hi elad,

thanks for the response and attempt. i commented out the overloaded unary increment operators i was using and inserted your code to test and see the results. the prefix increment operator worked without any problems, however, when using the postfix operator, the program experienced the same assert failures during run-time as i experienced when first posting the original question with the author-related eg. once i commented out the calls to the overloaded postfix unary increment operator the program did not have any execution problems. i honestly would like to have a book before me that covers just about any use of overloaded operators there are with multiple egs. and thorough explanations.
thanks again...

>>Counter temp(*this);
>>(...)
>>return temp;

You're returning a reference to a variable which immediately goes out of scope. Perhaps it would be better to return an actual Counter object instead - or even declare temp as a static or member variable, and use the = operator.

I've been thinking about how your author (you?)declared and defined the ++ operator. Your author only declared a single ++ operator, so they didn't have to differentiate between prefix and post fix. Whether the syntax for it's use would be ++game, as in prefix by my author, or game++, as in postfix by my author is unclear. And maybe it doesn't matter. I can't say.

Your author has declared the ++ operator as a friend operator/functionmethod of the FootballGame class rather than a member function of the football game class as did my author. This is okay because only the =, [], (), and -> must be members of the class. All other operators may be friends or members. As a friend method/function/operator the ++ operator isn't actually using the current object but must be passed an object to be used. They have indicated the argument should be a constant reference to a FootballGame object, because the body of ++operator won't change the object passed in, and it's quicker to pass by reference than by value. Your author's ++ operator will return a FootballGame object. This is probably okay because my author goes through a series of code for overloading the ++operator. One version has void as the return type, one has the class type (like your author does) as the return type, one has a reference to a class type, and the final recommendation was a const reference to class type, as I posted. EDIT: but he only did that for the prefix operator. He left the return type for the postfix operator at a cont type, not a reference or a const reference. Sorry.

Your author declares a variable of type FootballGame called game on the first line of the body of the ++operator. This object is local to the function and will be used as the return object. This object called game isn't the same object called game that was declared back in main() and passed to the ++operator, however. The local game was declared using the default constructor which assigned memory to each of the member variables. In the body of the ++operator in your code, each of member variables is assigned a second set of memory addresses by using the new operator, therefore leaking the memory originially assigned to the member varaibles when the default constructor was called.

The purpose of the ++ operator, appears to be to increase the iScore variable by 6 rather than by one, as is the usual use of the ++ operator. This is okay because it is legal to implement any operator as you see fit. However, the conventional use of the ++ operator, as you no doubt know, is to increase by one.

iScore is declared as a private static member variable of the FootballGame class but the ++operator is declared as a friend to the FootballGame class. Therefore, I'm not sure if you need to change/get the value of iScore with a public method or whether you can access it directly.

Overall, using your code based on your author's syntax, I guess I'd try something like:

Code:

//declaration as public friend operator
friend 
FootballGame operator ++(const FootballGame & operand);
 
//definition of friend operator
FootballGame operator ++(const FootballGame & operand)
{
  FootballGame game;
  game.setCityName(operand.getCityName());
  game.setTeamName(operand.getTeamName());
  game.setQBName(operand.getQBName());
  game.setMascotName(operand.getMascotName());
 
  game.setiScore(operand.getiScore() + 6);
 
  return game;
}

Overall, however, I'd be reluctant to try to teach overloading the ++ operator by implementing it on a class with a static member variable and a friend function. But then, as you can tell, I have enough trouble remembering how to overload the ++ operator(s) for a simple class!