Returning template map value as reference

Question

Well, I must say that working with C++ templates + stl made of templates + trying to do fast c++11 code is a pain. Mostly, lots of strange compiler errors... I need a help, code:

#include <SFML/Graphics.hpp>
#include <memory>
#include <map>

template <class T>
class cResourceManager
{
public:
T& get(const std::string & key);
bool add(const std::string & key, const std::shared_ptr<T> & ptr);
private:
std::map <std::string, std::shared_ptr<T> > resources;
};

template <class T>
T& cResourceManager<T>::get(const std::string & key)
{
    class std::map<std::string, std::shared_ptr<T>>::const_iterator citr =     resources.find(key);
    if (citr != resources.end()) return resources[key];
}

template <class T>
bool cResourceManager<T>::add(const std::string & key, const std::shared_ptr<T> & ptr)
{
if (resources.find(key) == resources.end())
{
    if(ptr != nullptr) 
    {
        resources.insert( std::move( std::make_pair(key, ptr) ) );
        return true; 
    }
}
return false;
}

int main(int argc, char **argv)
{
    cResourceManager<sf::Texture> resmgr;
    resmgr.add("key", std::make_shared<sf::Texture>() );
    resmgr.get("key");
    return 0;
}

On the line resmgr.get("key") i get an error "main.cpp:19:51: error: invalid initialization of reference of type ‘sf::Texture&’ from expression of type ‘std::map, std::shared_ptr, std::less >, std::allocator, std::shared_ptr > > >::mapped_type {aka std::shared_ptr}’" I have no idea why and trying to understand errors using templates and STL is extremely hard to me. I have no clue what's wrong.

The second thing is a little question. On the line: resources.insert(std::move(std::make_pair(key, ptr))) do I need the std::move function to get better performance? Because I want to avoid temporary objects when working with cointainers as much as possible but I don't think that I understand everything so I'm not sure about it.

Thanks!

What do you expect get() to return if there is no item already by that key? As it stands now, on there is NO return value, i.e. UB. — WhozCraig
There is an object with that key. look, there is add function line above. — user1873947
I'm not talking about the code that calls get(). I'm talking about the actual implementation of get(). — WhozCraig
If I don't use get I get no error in compilation. The error is only in usage. Anyway, so what's the solution? I don't exactly understand what's wrong. — user1873947
Its a template, so of course if you don't call it there is no error (because there is no expansion). You need to decide whether a failed key lookup is an error condition, because as it stands now you have undefined behavior in get() if the lookup fails. For example, a failed lookup could throw an exception and catch it in the caller. That function must return a valid T& as it is written now, and you don't currently do that in a failed lookup case. — WhozCraig

Vaughn Cato Vaughn Cato · Accepted Answer · 2012-12-10T15:41:10

The error is on this line:

if (citr != resources.end()) return resources[key];

resources[key] will give you a std::shared_ptr<T>, but your function returns a T &. You would need something like this instead:

if (citr != resources.end()) return *resources[key];

You also need to decide what you are going to do if the key isn't found. Currently the function doesn't return anything in that case.

As for your other question, make_pair returns a temporary pair, which is already an rvalue, so there is no need for an explicit move.

Returning template map value as reference

1 Answers