Program goes to destructor before using the object!!!

Collapse
X
 
  • Time
  • Show
Clear All
new posts
  • jewel87
    New Member
    • Jan 2007
    • 62

    Program goes to destructor before using the object!!!

    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(cla ss 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:

    Code:
    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...

    Code:
    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
    New Member
    • Sep 2007
    • 90

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

    Code:
    theEmployeeList.addEmployee(i, Size);

    Comment

    • jewel87
      New Member
      • Jan 2007
      • 62

      #3
      Originally posted by Cucumber
      Could you post the code of the following function? (addEmployee)

      Code:
      theEmployeeList.addEmployee(i, Size);
      Sure:

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

      Comment

      • RRick
        Recognized Expert Contributor
        • Feb 2007
        • 463

        #4
        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?

        Comment

        • Cucumber
          New Member
          • Sep 2007
          • 90

          #5
          Code:
          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:
          Code:
          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

          Comment

          • jewel87
            New Member
            • Jan 2007
            • 62

            #6
            Originally posted by Cucumber
            Code:
            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:
            Code:
            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...

            Comment

            • Cucumber
              New Member
              • Sep 2007
              • 90

              #7
              Originally posted by jewel87
              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:


              Code:
              // 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

              Comment

              • jewel87
                New Member
                • Jan 2007
                • 62

                #8
                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<*Employe e>

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

                Comment

                • Cucumber
                  New Member
                  • Sep 2007
                  • 90

                  #9
                  Originally posted by jewel87
                  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<*Employe e>

                  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.

                  Comment

                  • weaknessforcats
                    Recognized Expert Expert
                    • Mar 2007
                    • 9214

                    #10
                    Originally posted by jewel87
                    void EmployeeList :: addEmployee(int &i, int &SIZE)
                    {
                    i = SIZE;
                    myEmployeeList. push_back(Emplo yee()); <<<<<<
                    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.

                    Comment

                    Working...