Move semantics for a resource manager class

Question

I am trying to make a resource class for my game (which makes use of the SFML API). Basically I first load the needed resources and then I just get references to them when needed in order to avoid the heavy construction of the resource classes.

(I will use the sf::Texture class as an example)

The problem lies in the load function. In order to make an sf::Texture I have to use the default constructor and then use it's loadFromFile function to get the needed texture. Say I have a map containing pairs of the file names and the respective resources:

std::map<std::string, sf::Texture> textures;
sf::Texture texture;
texture.loadFromFile(file);

Now if I do:

textures.emplace(file, texture);

This will use the copy-constructor to make another sf::Texture. So I thought I should use std::move but apparently the sf::Texture class doesn't have a move constructor so this would still make a copy. In order to solve this I am now using a map containing pairs of the file names and the respective unique_pointers to the objects. So I do:

std::unique_ptr<sf::Texture> texture(new sf::Texture);
texture->loadFromFile(file);
textures.emplace(file, std::move(texture));

Here is how the complete function looks like:

void ResourceManager::loadTexture(std::string file)
{
        std::unique_ptr<sf::Texture> texture(new sf::Texture);
        if (!texture->loadFromFile(file))
        {
                throw std::runtime_error("ResourceManager::load - Failed to load " + file);
        }
        auto inserted = mtextures.emplace(file, std::move(texture));
        assert(inserted.second);
}

I don't have much experience with move semantics though and I am not sure if I got everything right. Basically, have I missed anything? If this question does not belong to stack overflow I will gladly move it to the appropriate site.

Answer 1

The code you've posted looks correct, given the description of what you're trying to do.

The map will be designated as the owner of the resource. When it is destroyed, the resource will be destroyed as well, unless something has retrieved the resource (e.g., via a std::move) beforehand.

The only suggestion I would make its to use std::make_unique when creating the pointer (for exception-safety, and to make the code more idiomatic). Other than that minor detail, it sounds like you've got the right idea.

Answer 2

If you're concerned that you can't efficiently move an sf::Texture, then don't move an sf::Texture. Construct them directly in the map:

void ResourceManager::loadTexture(const std::string& file)
{
    assert(mtextures.find(file) == mtextures.end());
    auto& t = mtextures[file];
    try {
        if (!t.loadFromFile(file)) {
            throw std::runtime_error("ResourceManager::load - Failed to load " + file);
        }
    } catch(...) {
        mtextures.erase(file);
        throw;
    }
}