Avihai Marchiano
Avihai Marchiano

Reputation: 3927

Do i need to delete objects that i set in class member when i replace them?

I have the following:

class Manager{
public:
    void update(list<Employe> employees){
        employees_ = employees;
    }
private:
    list<Employe> employees_;
};

do i need to delete old employees in the end of update method?

Upvotes: 0

Views: 90

Answers (4)

Sarfaraz Nawaz
Sarfaraz Nawaz

Reputation: 361352

In C++11, I would suggest you to use std::move as:

void update(list<Employee> employees) //NOTE :  pass by value
{
    employees_ = std::move(employees);
}

Note that the argument is passed by value, not passed by reference. This means, when you write this,

update({e1,e2,e3,e3});

then the temporary list created from argument, is moved to the member variable. In this way, you avoid making a copy of the temporary object (which is going to get destroyed, anyway).

If you use list<Employee> const & employees as parameter, then you wouldn't be able to std::move the resource of the temporary object created from the above call.

Upvotes: 0

Spook
Spook

Reputation: 25927

There is no "old employees". In this case a

list<Employee>::operator = (const list<Employee> & source)

will be called. If you didn't defined one, the default one will copy raw contents of the instance passed as a parameter (employees) to the field (employees_).

Now suppose, that list contains a pointer to a dynamically allocated memory. In such case a reference to that memory will be lost and it will leak.

The correct solution is either to check, if operator = is overloaded correctly (for example all standard containers already has it implemented) or implement it by yourself (pseudocode):

void list<Employee>::operator = (const list<Employee> & source)
{
    freeContents();
    for (int i = 0; i < source.size(); i++)
        add(source.getItem(i));
}

Edit:

If the list is actually a std::list, it will handle the assignment correctly, so in this case the answer is: yes, the list itself will be freed automatically. Its contents, however, that's another story.

Upvotes: 2

Andrew
Andrew

Reputation: 24846

No, you don't, they will be destroyed automatically. Because they are stored on automatic storage (stack)

Also a little improvement:

void update(const list<Employe> & employees){ //pass by reference, not by value
    employees_ = employees; //old values are automatically destroyed and 
   // copies of new values are added, preserving the original values in passed container
}

Upvotes: 0

ecatmur
ecatmur

Reputation: 157334

In C++, the assignment operator copies the LHS to the RHS, and takes care of destroying whatever is currently occupying the LHS.

In the code

void update(list<Employe> employees){
    employees_ = employees;
}

after the function has executed, the previous contents of employees_ will have been destroyed, and employees_ will now contain a copy of the parameter employees.

To make this more efficient, you can eliminate the copy:

employees_ = std::move(employees);    // C++11

or

std::swap(employees_, employees);     // C++03

In the first case, the contents of employees_ will be discarded, and the contents of employees will be moved to employees_, leaving employees empty.

In the second case, the contents of employees_ and employees will be swapped, so that when the function returns the original contents of employees_ will be discarded.

Upvotes: 1

Related Questions