Reuven Guetta
Reuven Guetta

Reputation: 47

Cannot erase a shared_ptr from set

I am trying to have an object with a set of pointers to another object. when I try to erase on of the set's values I get an error and crash, I really dont know what could be causing it. here is the library and after that the main function: when I try to run it it does everything its supposed to do, and when it gets to the removeemployee it crashes and sends out the following: Process finished with exit code -1073740940 (0xC0000374)

I run it on clion if that matters, and in c++11.

#include <ostream>
#include <iostream>
#include "Manager.h"

Manager::Manager(int id, string firstName, string lastName, int birthYear)
        : Citizen(id, firstName, lastName,birthYear), salary(0), employees(), work_flag(false) {}

int Manager::getSalary() const {
    return salary;
}

void Manager::setSalary(int _salary) {
    if((salary + _salary) < 0){
        salary = 0;
    }else {
        salary += _salary;
    }
}

void Manager::addEmployee(Employee* employee_add) {
    shared_ptr<Employee> employee(employee_add);
    if(employees.find(employee) != employees.end()){
        throw mtm::EmployeeAlreadyExists();
    }
    employees.emplace(employee);
}

//this is the function

void Manager::removeEmployee(int id) {
    for(auto it = employees.begin(); it != employees.end(); it++){
        if(it->get()->getId() == id){
            employees.erase(it);
            return;
        }
    }
    throw mtm::EmployeeDoesNotExists();
}

Manager *Manager::clone() {
    return new Manager(*this);
}

ostream &Manager::printShort(ostream &os) const {
    os<<this->getFirstName()<<" "<<this->getLastName()<<endl;
    os<<"Salary :"<<this->getSalary()<<endl;
    return os;
}

ostream &Manager::printLong(ostream &os) const {
    os<<this->getFirstName()<<" "<<this->getLastName()<<endl;
    os<<"id - "<<this->getId()<<" birth_year - "<<this->getBirthYear()<<endl;
    os<<"Salary :"<<this->getSalary()<<endl;
    os<<"Employees:"<<endl;
    for(const auto & employee : employees){
        employee->printShort(os);
    }
    return os;
}

bool Manager::findEmployee(int id) {
    int i = 0;
    for(const auto & employee : employees){
        cout<<++i<<endl;
        if(employee->getId() == id){
            cout<<"return true"<<endl;
            return true;
        }
    }
    cout<<"return false"<<endl;
    return false;
}

bool Manager::isWorkFlag() const {
    return work_flag;
}

void Manager::setWorkFlag(bool workFlag) {
    work_flag = workFlag;
}

and this is the main function:

int main() {
    Employee e1(1, "John", "Williams", 2002);
    Employee e2(2, "Alex", "Martinez", 2000);
    Manager m1(1,"Robert", "stark", 1980);
    m1.addEmployee(&e1);
    m1.addEmployee(&e2);
    Employee e3(7, "Reuven", "Guetta", 2001);
    m1.addEmployee(&e3);
    m1.printLong(cout);
    cout<<"Delete"<<endl;
    //here is the problem
    m1.removeEmployee(e2.getId());
    m1.printLong(cout);
    return 0;
}

Upvotes: 1

Views: 119

Answers (1)

Sam Varshavchik
Sam Varshavchik

Reputation: 118340

shared_ptr<Employee> employee(employee_add);

There is only one reason to have a shared_ptr, in the first place; there's only one reason for its existence; it has only one mission in its life, as explained in every C++ textbook: to be able to new an object, and have the shared_ptr automatically take care of deleteing it when all references to the object are gone, avoiding a memory leak.

In your program this object was not instantiated in dynamic scope with new:

    Employee e2(2, "Alex", "Martinez", 2000);
    Manager m1(1,"Robert", "stark", 1980);
    m1.addEmployee(&e1);

    // etc, etc, etc...

and that's the reason for the crash.

If you are not using new, simply get rid of all shared_ptrs in the shown code.

Upvotes: 4

Related Questions