C++ Storing and passing references instead of values in an unordered_list

Question

I have some code which compiles and runs fine, but passes the objects by value:

AssetRepository AssetRepositoryFactory::getAssetRepository(std::string clientId)
{
    std::unordered_map<std::string, AssetRepository>::iterator iterator = m_repositories.find(clientId);
    if ( iterator != m_repositories.end())
    {
        return iterator->second;
    }
    else
    {
        AssetRepository repository = AssetRepository(clientId);
        std::pair<std::string, AssetRepository> pair (clientId, repository);
        m_repositories.insert(pair);
        return repository;
    }

}

What I intended this code to do was pass by reference, but this is probably a fairly common newbie C++ developer mistake. So I tried to pass by reference by changing to this code:

AssetRepository& AssetRepositoryFactory::getAssetRepository(std::string clientId)
{
    std::unordered_map<std::string, AssetRepository&>::iterator iterator = m_repositories.find(clientId);
    if ( iterator != m_repositories.end())
    {
        return iterator->second;
    }
    else
    {
        AssetRepository repository = AssetRepository(clientId);
        std::pair<std::string, AssetRepository&> pair (clientId, repository);
        m_repositories.insert(pair);
        return repository;
    }

}

Unfortunately I get this error:

HEAP[CGF_flight_controller.exe]: Invalid allocation size - c3d9e9e0 (exceeded fffdefff)
First-chance exception at 0x000007FEFD24B3DD in CGF_flight_controller.exe: Microsoft C++ exception: std::bad_alloc at memory location 0x0000000002C6F4F0.
Microsoft Visual Studio C Runtime Library has detected a fatal error in CGF_flight_controller.exe.

That's if I call it via

AssetRepository repository = AssetRepositoryFactory::getInstance().getAssetRepository("clientId0");

Or, if I call

AssetRepository &repository = AssetRepositoryFactory::getInstance().getAssetRepository("clientId0");

I get

First-chance exception at 0x000007FEE72EAD7B (RTDynamics-vc11-md-64.dll) in CGF_flight_controller.exe: 0xC0000005: Access violation reading location 0x0000000000000028.
Unhandled exception at 0x000007FEE72EAD7B (RTDynamics-vc11-md-64.dll) in CGF_flight_controller.exe: 0xC0000005: Access violation reading location 0x0000000000000028.

I'm having trouble getting my head around having to explicitly pass by reference as I am normally a Java developer. Please can someone assist me to figure it out?

I'm going to guess that it's got do with the definition of the class `AssetRepository`. Please post that. — R Sahu, Jan 14 '16 at 03:37

score 0 · Answer 1 · answered Jan 14 '16 at 03:42

This code is absolutely wrong.

AssetRepository& function() {
    [...]
    AssetRepository repository = [...];
    [...]
    return repository; // by reference
    // but "repository" gets destroyed...
}

You are returning a reference to an object that doesn't exist after the function returns. Don't do that. This is called a "dangling reference" (put that into Google). You can only use references to objects that exist.

score 0 · Answer 2 · edited May 23 '17 at 11:44

The problem is here:

std::unordered_map<std::string, AssetRepository&>

The value of containers should be Assignable. but references are not assignable. you can only initialize them once when they are declared, and you cannot make them reference something else later.

But you have some solutions:

Use values as your working code is doing. and use move semantics to reduce cost of copying values.
Use a pointer to AssetRepository in your map:

std::unordered_map<std::string, AssetRepository*>
Use reference wrapper to make the reference assignable.

For more info and solutions, check this question: Why can't I make a vector of references?

score 0 · Answer 3 · answered Jan 14 '16 at 05:03

I found a solution in the end, and incorporated both the comment about a dangling reference and the suggestion to use pointers instead.

AssetRepository* AssetRepositoryFactory::getAssetRepository(std::string clientId)
{

    std::unordered_map<std::string, AssetRepository>::iterator iterator = m_repositories.find(clientId);
    if ( iterator != m_repositories.end())
    {
        AssetRepository repository = iterator->second;
        return &(iterator->second);
    }
    else
    {
        AssetRepository repository = AssetRepository(clientId);
        std::pair<std::string, AssetRepository> pair (clientId, repository);
        m_repositories.insert(pair);
        return getAssetRepository(clientId);
    }

}

C++ Storing and passing references instead of values in an unordered_list

3 Answers3