Program goes to destructor before using the object!!!

[jewel87]

Hi,
I have a program with several classes which i was debugging and just found out strange thing: it creates the object in a constructor(class Employee), allocates all memory for other class objects, and then goes straight to the destructor and deallocates all memory just allocated. Only then it goes to the function using this object and gives errors because the memory has just been deallocated.
Why is it doing so????
This is the class implementation:

void Employee :: create(TEdit *EnterEmployeeName,
                        TEdit *EnterEmployeeSurname,
                        TEdit *EnterEmployeeSocialNumber,
                        TEdit *EnterEmployeeSalary,
                        TEdit *EnterEmployeeHouseName,
                        TEdit *EnterEmployeeHouseNo,
                        TEdit *EnterEmployeeStreet,
                        TEdit *EnterEmployeeTown,
                        TEdit *EnterEmployeePostCode,
                        TEdit *EnterEmployeeCountry,
                        TEdit *EnterDOBdate,
                        TEdit *EnterDOBmonth,
                        TEdit *EnterDOByear,
                        TEdit *EnterEmploymentStartDate,
                        TEdit *EnterEmploymentStartMonth,
                        TEdit *EnterEmploymentStartYear,
                         TEdit *EnterBankCode,
                         TEdit *EnterAccountNo,
                         TEdit *EnterBalance,
                         TEdit *EnterBankName,
                         TEdit *EnterRate,
                         TEdit *EnterOverdraftLimit,
                         TEdit *EnterConditions)
{
        //save data for the employee
        FirstName = EnterEmployeeName->Text;
        Surname = EnterEmployeeSurname->Text;
        Salary = StrToInt(EnterEmployeeSalary->Text);
        SocialNumber = StrToFloat(EnterEmployeeSocialNumber->Text);
 
       HomeAddress -> create(EnterEmployeeHouseName->Text,
                                StrToInt(EnterEmployeeHouseNo->Text),
                                EnterEmployeeStreet->Text,
                                EnterEmployeeTown->Text,
                                EnterEmployeePostCode->Text,
                                EnterEmployeeCountry->Text);
        DOB->setDate(StrToInt(EnterDOBdate->Text),
                        StrToInt(EnterDOBmonth->Text),
                        StrToInt(EnterDOByear->Text));
 
        EmploymentStartDate->setDate(StrToInt(EnterEmploymentStartDate->Text), StrToInt(EnterEmploymentStartMonth->Text), StrToInt(EnterEmploymentStartYear->Text));
 
       theAccounts[0]->create(EnterBankCode->Text,
                                StrToInt(EnterAccountNo->Text),
                                StrToFloat(EnterBalance->Text),
                                EnterBankName->Text,
                                StrToFloat(EnterRate->Text),
                                StrToFloat(EnterOverdraftLimit->Text),
                                EnterConditions->Text);   
}
 
Employee :: Employee()
{
        FirstName = "";
        Surname = "";
        SocialNumber = -1;
        Salary = -1;
        DOB = new clsDate();
        EmploymentStartDate = new clsDate();
 
        HomeAddress = new Address();
 
        theAccounts = new Account*[3];
         theAccounts[0] = new CurrentAccount();
}
 
 
Employee :: ~Employee()   /////program goes here straight from the constructor
{
 
            delete HomeAddress;
            HomeAddress = NULL;
 
 
        delete DOB;
       delete EmploymentStartDate;
 
        delete theAccounts[0];
        delete theAccounts;
}
 
 
 

And this is the function in my Form where i call this Employee class function create(), which is preformed after the destructor...

 
void __fastcall TForm1::CreateEmployeeButtonClick(TObject *Sender)
{
        theEmployeeList.addEmployee(i, Size);
 
        theEmployeeList.getEmployee(i)->create(EnterEmployeeName,
                                              EnterEmployeeSurname,
                                              EnterEmployeeSocialNumber,
                                              EnterEmployeeSalary,
                                              EnterHouseName,
                                              EnterHouseNo,
                                              EnterStreet,
                                              EnterTown,
                                              EnterPostCode,
                                              EnterCountry,
                                              EnterDOBdate,
                                                EnterDOBmonth,
                                                EnterDOByear,
                                                EnterEmploymentStartDate,
                                                EnterEmploymentStartMonth,
                                                EnterEmploymentStartYear,
                                                EnterBankCode,
                                                EnterAccountNo,
                                                EnterBalance,
                                                EnterBankName,
                                                EnterRate,
                                                EnterOverdraftLimit,
                                                EnterConditions);
 
 

Could someone please help...

[Cucumber]

Could you post the code of the following function? (addEmployee)

theEmployeeList.addEmployee(i, Size);
 

[jewel87]

Could you post the code of the following function? (addEmployee)

theEmployeeList.addEmployee(i, Size);
 

Sure:

 
void EmployeeList :: addEmployee(int &i, int &SIZE)
{
        i = SIZE;
        myEmployeeList.push_back(Employee());
        SIZE++;
        ListSize = SIZE;
}
 
 

[RRick]

What you are describing is the behavior between a base class and a derived class where the derived class' constructor throws an exception. In this case, the base class desctructor will be called.

The STL library uses this design where the base class handles memory allocation, and if the derived class blows up, the memory is guaranteed to be deallocated.

Since you haven't posted the class definition, does this match your situation?

[Cucumber]

myEmployeeList.push_back(Employee());
 

Im assuming this is a list (although it could be a vector, doesnt matter).

This code creates an object in the stack, at that moment the constructor is called.

Stack objects die when the function they are created in finishes (strictly speaking they die when the scope whithin they were created in finishes), so you see the destructor being called as a result of that; i.e. when addEmployee finishes the destructor gets called.

The STL list will create a copy of your stack object.

I am assuming you dont have neither a copy constructor nor an overloaded = operator in your Employee class, so the copy STL is doing is a bad copy, since you have pointers inside your Employee class, and when you try using them the program crashes because you have already disposed those pointers in the destructor.

So to solve this problem, create a copy constructor and overload the operator= of your Employee class, just make sure to deal with pointers accordingly inside both the copy constructor and the overladed = operator.

OR

have your list use heap objects rather than stack objects and use this:

myEmployeeList.push_back(new Employee());
 

In the latter case, make sure to free all the pointers inside the list when needed.

by the way, it is not necessary to keep a count of how many employees you have entered into the list, the function count() will give you the current list size; or is it size()? I cant remember :S

[jewel87]

myEmployeeList.push_back(Employee());
 

Im assuming this is a list (although it could be a vector, doesnt matter).

This code creates an object in the stack, at that moment the constructor is called.

Stack objects die when the function they are created in finishes (strictly speaking they die when the scope whithin they were created in finishes), so you see the destructor being called as a result of that; i.e. when addEmployee finishes the destructor gets called.

The STL list will create a copy of your stack object.

I am assuming you dont have neither a copy constructor nor an overloaded = operator in your Employee class, so the copy STL is doing is a bad copy, since you have pointers inside your Employee class, and when you try using them the program crashes because you have already disposed those pointers in the destructor.

So to solve this problem, create a copy constructor and overload the operator= of your Employee class, just make sure to deal with pointers accordingly inside both the copy constructor and the overladed = operator.

OR

have your list use heap objects rather than stack objects and use this:

myEmployeeList.push_back(new Employee());
 

In the latter case, make sure to free all the pointers inside the list when needed.

by the way, it is not necessary to keep a count of how many employees you have entered into the list, the function count() will give you the current list size; or is it size()? I cant remember :S

Could you please explain briefly what is a copy constructor and why will it help in this case? and what is operator =, how do i overload it? I've tried to use myEmployeeList.push_back(new Employee());
but still can't make it work... i'm so confused now and don't have much time left...

[Cucumber]

Could you please explain briefly what is a copy constructor and why will it help in this case? and what is operator =, how do i overload it? I've tried to use myEmployeeList.push_back(new Employee());
but still can't make it work... i'm so confused now and don't have much time left...

I would like to help but seems like doing so properly would require to take a look to many of your classes. I am going to try to assume a few things to help you:

// This is a copy constructor, notice you pass an Employee object to
// the constructor, the point is making an exact copy
Employee :: Employee( const Employee &employee )
{
        FirstName = employee.FirstName; // I assume this is a string object
        Surname = employee.Surname; // I ssume this is a string object too
        SocialNumber = employee.SocialNumber;
        Salary = employee.Salary;
 
       // this is where things become icky, you now must have
       // copy constructors for clsDate, Address, and Account
 
        DOB = new clsDate( *employee.DOB );
        EmploymentStartDate = new clsDate( *employee.EmploymentStartDate );
 
        HomeAddress = new Address( *employee.HomeAddress );
 
        theAccounts = new Account*[3];
         theAccounts[0] = new CurrentAccount( *employee.theAccounts[0] );
}
// this is the operator= it is used heavily by STLs
Employee & Employee::operator=( const Employee &employee )
{
   if(this==&employee)
      return *this;
 
            delete HomeAddress;
            HomeAddress = NULL;
 
 
        delete DOB;
       delete EmploymentStartDate;
 
        delete theAccounts[0];
        delete theAccounts;
 
        FirstName = employee.FirstName; // I assume this is a string object
        Surname = employee.Surname; // I ssume this is a string object too
        SocialNumber = employee.SocialNumber;
        Salary = employee.Salary;
 
       // this is where things become icky, you now must have
       // copy constructors for clsDate, Address, and Account
 
        DOB = new clsDate( *employee.DOB );
        EmploymentStartDate = new clsDate( *employee.EmploymentStartDate );
 
        HomeAddress = new Address( *employee.HomeAddress );
 
        theAccounts = new Account*[3];
         theAccounts[0] = new CurrentAccount( *employee.theAccounts[0] );
   return *this;
}
 

As you can see now you need copy constructors for Address, clsDate and CurrentAccount.

If you are in a hurry use a list of heap objects.
list< *Employee > instead of
list< Employee >
so you would avoid all this copy constructor thing.

Regards

[jewel87]

If you are in a hurry use a list of heap objects.
list< *Employee > instead of
list< Employee >
so you would avoid all this copy constructor thing.

Regards

Thank you very much for help, i'll try to study copy constructor thing when i get a bit of time.
i'm using vectors in this case, would

vector<*Employee>

mean that i create a vector of pointers to Employee objects?

[Cucumber]

Thank you very much for help, i'll try to study copy constructor thing when i get a bit of time.
i'm using vectors in this case, would

vector<*Employee>

mean that i create a vector of pointers to Employee objects?

Correct.


As a side note, you would need to take a look into getEmployee when doing that change.

[weaknessforcats]

void EmployeeList :: addEmployee(int &i, int &SIZE)
{
i = SIZE;
myEmployeeList.push_back(Employee()); <<<<<<
SIZE++;
ListSize = SIZE;
}

Why are you calling constructors???? There is no object.

You should have:
[code=cpp]
Employee temp;
myEmployeeList.push_back(temp);
[/quote]

The Employee() creates a temporary object that is copied to the vector.

I assume you have a proper copy constructor and aren't doing something foolish like copying pointers. If you are, when the temporary object is deleted and the destructor called, the data for the object in the vector will be deleted also. Be sure your copy constrcutor actually makes a copy of the data members.