1

I'm trying to make map in which i will hold Teams as key and vector of Employees which are polymorphic as value. Some of the data will be loaded from file in future and the user will be able to add new teams and employees at any time.

This is the map that i came up with:

std::map<std::unique_ptr<Team>, std::unique_ptr<std::vector<std::unique_ptr<Employee>>>> teams;

And this is some test code where i tried to add new team and a member to it:

bool Company::addTeam(const std::string & projectName)
{
    teams.emplace(std::unique_ptr<Team>(new Team(projectName)), std::unique_ptr<std::vector<std::unique_ptr<Employee>>>(new std::vector<std::unique_ptr<Employee>>()));
    for (std::map<std::unique_ptr<Team>, std::unique_ptr<std::vector<std::unique_ptr<Employee>>>>::iterator it = teams.begin(); it != teams.end(); ++it) {
        std::cout << it->first.get()->getProjectName();
        it->second.get()->emplace_back(std::unique_ptr<Employee>(new Programmer("David", "Beckham", "9803268780", "Djoe Beckham", Employee::LevelThree, projectName)));
        std::cout << "\n" << it->second.get()->at(0)->toString();
    }


    return false;
}

The code runs fine and im able to print the employee data but after closing the application it throws exception and visual studio open delete_scalar.cpp and triggers a breakpoint at this code :

void __CRTDECL operator delete(void* const block) noexcept
{
    #ifdef _DEBUG
    _free_dbg(block, _UNKNOWN_BLOCK); // break point
    #else
    free(block);
    #endif
}

I'm trying to find out what i'm doing wrong and how to fix it. I have empty destructors for all Employees classes if that have something to do with the problem. If my idea looks very stupid and there is easier way to accomplish my goal please tell me. Thanks in advance.

Christophe
  • 68,716
  • 7
  • 72
  • 138
Ivaylo Hristov
  • 183
  • 11
  • 2
    Are you sure `std::map>` or even `std::multimap` wouldn't do the job? – François Andrieux May 07 '19 at 18:14
  • Instead of overriding the `delete` operator you could use a custom deleter used with the `std::unique_ptr`s. – πάντα ῥεῖ May 07 '19 at 18:16
  • The code you've shown doesn't seem to explain the error you are asking about. I suspect that there is a problem with `Team` or `Employee` that is causing this problem. To be certain, you should provide a [MCVE]. – François Andrieux May 07 '19 at 18:17
  • @πάνταῥεῖ My understanding of the question is that the shown `delete` is just where Visual Studio is claiming the error is occurring. I don't see where the question claims that they wrote this `delete`. Edit : I get the same thing if I `delete (char*)1;` – François Andrieux May 07 '19 at 18:18
  • What have you done so far to debug the problem? In a case like this, where the problem manifests in the cleanup, it can be very helpful to delete (or temporarily comment out) portions of the code to try to narrow down the area of code which causes the problem. And see https://ericlippert.com/2014/03/05/how-to-debug-small-programs/ – Basya May 07 '19 at 18:50

2 Answers2

1

Design-wise, one person can have multiple roles in different teams at the same time. It can be another class Role linking a person to a team. Neither team, no role conceptually own person objects though, so Role could use plain pointers to link Person with Team.

Maxim Egorushkin
  • 131,725
  • 17
  • 180
  • 271
1

Your idea could work, if you would not have a problem within one of your class. Due to the missing parts, it's difficult to tell.

However, this is not the way to go !!

A map is meant to work with an index by value. A typical use case is to find back an item with an already existing key. Of course, you could use pointers as key, but the pointer would then act as a kind of id without any polymorphic operations.

On the other side, a unique_ptr is designed to ensure unique ownership. So only one unique copy of each pointer value exist. This makes it very difficult to use as map value: 

auto team2 = make_unique<Team>(); 
teams [team2] = make_unique<vector<unique_ptr<Employee>>>();  // doesn't compile

The above code doesn't compile, because team2 is a unique_ptr and cannot be copied into the indexing parameter. Using it for searching or inserting an item, would require to move it: 

teams [std::move(team2)] = make_unique<vector<unique_ptr<Employee>>>(); //compiles
assert (team2); // ouch

But once moved, the unique_ptr value is no longer in team2 which is now empty since the unique pointer is in the map key and it's unique. This means that you will never ever find back an added team.

Better alternatives ?

If you would want to really use a polymorphic pointer as a map key, you should at least use a shared_ptr, so that several copies can exist in your code. But I'd suggest that you use values only as a key

Now to the value part of the map. There is no benefit of making a unique_ptr of a vector: the vector itself is not polymorphic, and vectors are well designed for copying, moving and so on. Furthermore sizeof(vector<...>) is small even for very large vectors.

Use a vector<unique_ptr<Employee>> if you want the vector in the map to own the Employees, or vector<shared_ptr<Employee>> if you intend to share the content of the vector.

Christophe
  • 68,716
  • 7
  • 72
  • 138