c++ - runtime error: vector push_back()

Question

Please tell me reason why this code have runtime error. I think that addEmployee() function is problem.

#include <iostream>
#include <vector>
#include <string>
using namespace std;

enum EmployeeLevel {fresh, sophomore, junior, senior};

class Employee {
    string name;
    EmployeeLevel level;
public:
    Employee(const string & _name, const EmployeeLevel _level)
        : name(_name) {
        level = _level;
    }
    Employee(const Employee & employee) {
        name = employee.name;
        level = employee.level;
    }
    void changeLevel() {
        level = static_cast<EmployeeLevel> (level + 1);
    }
};

class Manager: public Employee {
    vector<Employee *> group;
public:
    Manager(const string & _name, const EmployeeLevel _level)
        : Employee(_name, _level) {}
    ~Manager() {
        for (vector<Employee *>::iterator it = group.begin(); it != group.end(); ++it)
            delete *it;
    }
    void addEmployee(Employee * employee) {
        group.push_back(employee);
    }
};

int main(void)
{
    Employee e1("홍", fresh), e2("김", sophomore), e3("차", fresh);    

    Manager m1("Tom", senior);
    m1.addEmployee(&e1);
}

If I modify the function(addEmployee()) as shown below, not error. But I want to know reason why using "push_back(employee)" has runtime error.

void addEmployee(Employee * employee) {
    Employee * tempEmployee = new Employee(*employee);
    group.push_back(tempEmployee);
}

Not your problem _now_, but the default copy constructor of `Manager` will copy the _pointers_ in `group`. That's bad, because `~Manager` will then delete them - twice. Solution: `Manager(Manager const&) = delete` removes that default copy ctor. — MSalters, Jun 14 '17 at 08:05

score 0 · Accepted Answer · answered Jun 14 '17 at 08:01

The problem with the first version, that crashes, is that you add a pointer to a local variable, i.e. an object that the compiler manages the storage for. That means when the Manager gets destructed it will try to delete this object, an object you haven't allocated with new.

Only delete what you new (and delete[] what you new[])!

In fact, I see no reasons to have a pointer at all. Instead have a vector of plain objects, i.e. std::vector<Employee>.

c++ - runtime error: vector push_back()

1 Answers1