CODEWITHSUNDEEP
c++c++11
Neil Kirk
Neil Kirk

Reputation: 21803

Is this the correct way to use move references and unique_ptr?

I've barely used the C++ move feature, so I'm not confident what I am doing is correct. I would appreciate anyone looking over my code and pointing out any mistakes I've made.

The idea is to create a map of resources stored by key. The resources may be non-copyable and non-movable.

Also, does my class need a constructor and destructor definition?

Thanks.

#define TYPE(x) std::identity<decltype(x)>::type

namespace General
{
    template<class T>
    std::string ToString(const T& x)
    {
        std::ostringstream ss;
        ss << x;
        return ss.str();
    }
}

namespace General
{
    template<class T, class KEY = std::string>
    class ResourceManager
    {
    public:
        typedef T ResourceType;
        typedef KEY KeyType;

        void Load(const KeyType& key, std::unique_ptr<ResourceType>&& resource);
        const ResourceType& Read(const KeyType& key) const;
        ResourceType& Modify(const KeyType& key);
        void Unload(const KeyType& key);
        std::unique_ptr<ResourceType>&& Release(const KeyType& key);
        void UnloadAll();

    private:
        std::map<KeyType, std::unique_ptr<ResourceType>> data;
    };
}

template<class T, class KEY>
void General::ResourceManager<T, KEY>::Load(const KeyType& key, std::unique_ptr<ResourceType>&& resource)
{
    auto find_it = data.lower_bound(key);
    if (find_it != data.end() && ! (data.key_comp()(key, find_it->first)))
    {
        throw std::runtime_error(General::ToString(key) + " already exists!");
    }
    else
    {
        data.insert(find_it, TYPE(data)::value_type(key, std::move(resource)));
    }
}

template<class T, class KEY>
const typename General::ResourceManager<T, KEY>::ResourceType& General::ResourceManager<T, KEY>::Read(const KeyType& key) const
{
    auto find_it = data.find(key);
    if (find_it == data.end())
    {
        throw std::runtime_error(General::ToString(key) + " could not be found!");
    }
    else
    {
        return *find_it->second;
    }
}

template<class T, class KEY>
typename General::ResourceManager<T, KEY>::ResourceType& General::ResourceManager<T, KEY>::Modify(const KeyType& key)
{
    auto find_it = data.find(key);
    if (find_it == data.end())
    {
        throw std::runtime_error(General::ToString(key) + " could not be found!");
    }
    else
    {
        return *find_it->second;
    }
}

template<class T, class KEY>
void General::ResourceManager<T, KEY>::Unload(const KeyType& key)
{
    auto find_it = data.find(key);
    if (find_it == data.end())
    {
        throw std::runtime_error(General::ToString(key) + " could not be found!");
    }
    else
    {
        data.erase(find_it);
    }
}

template<class T, class KEY>
std::unique_ptr<typename General::ResourceManager<T, KEY>::ResourceType>&& General::ResourceManager<T, KEY>::Release(const KeyType& key)
{
    auto find_it = data.find(key);
    if (find_it == data.end())
    {
        throw std::runtime_error(General::ToString(key) + " could not be found!");
    }
    else
    {
        auto resource = std::move(find_it->second);
        data.erase(find_it);
        return std::move(resource);
    }
}

template<class T, class KEY>
void General::ResourceManager<T, KEY>::UnloadAll()
{
    data.clear();
}

Upvotes: 3

Views: 325

Answers (1)

Kerrek SB
Kerrek SB

Reputation: 477378

Here's a simplified piece of code that demonstrates the crux of your situation and how to write the code idiomatically:

std::map<int, std::unique_ptr<Foo>> m;

void add_to_map(int key, std::unique_ptr<Foo> val)
{
    m[key] = std::move(val);
}

Usage:

add_to_map(1, std::unique_ptr<Foo>(new Foo(1, 2, 3)));

std::unique_ptr<Foo> p(new Foo(true, 'x'));
p->mutate();
add_to_map(std::move(p));

Basically, pass the unique pointer (or any other movable-only type, for that matter) by value and move from it.

One special situation I've encountered is when you want to take ownership of an object conditionally. In that case, pass the unique pointer by reference and inspect it afterwards:

void add_maybe(std::unique_ptr<Foo> & val)
{
    if (rand() % 2 == 0)
    {
        m[12] = std::move(val);
    }
}

Usage:

std::unique_ptr<Foo> p(new Foo(true, 'x'));

add_maybe(p);

if (p) { /* we still own the resource            */ }
else   { /* the resource is now owned by the map */ }

Update: To release an object from the map, return by value:

std::unique_ptr<Foo> release(int key)
{
    auto it = m.find(key);
    return it == m.end() ? { } : std::move(it->second);
}

Upvotes: 3

Related Questions