Allocating and managing OpenGL-based objects

Recently, I added this class to my Spiky engine: its basic purpose is to allocate OpenGL-based objects (such as Textures, Shader, Fonts, ...) and then manage them by giving them an ID so the user can pull objects in and out. Any remarks, suggestions or other comments are welcome.

Resourcemanager.h:

```
#pragma once
#include
#include
#include
#include "../render/Shader.h"
#include "../render/Mesh.h"
#include "../render/Texture.h"
#include "../render/Font2D.h"

namespace Spiky
{
class ResourceManager;

void inline InitSpikyCore();

class ShaderImp
{
public:
friend class ResourceManager;
friend std::unique_ptr::deleter_type;

std::unique_ptr const& operator->() const
{
return m_shader;
}
private:
explicit ShaderImp(const char vs, const char fs)
:
m_shader(std::make_unique(vs, fs))
{
}

explicit ShaderImp(const char vs, const char fs, const char* gs)
:
m_shader(std::make_unique(vs, fs, gs))
{
}

~ShaderImp()
{
}

std::unique_ptr m_shader;
};

typedef ShaderImp const& Shader;

class MeshImp
{
public:
friend class ResourceManager;
friend std::unique_ptr::deleter_type;

std::unique_ptr const& operator->() const
{
return m_mesh;
}

private:
explicit MeshImp(Vertex vertices, unsigned int numVertices, unsigned int indeces, unsigned int numIndices)
:
m_mesh(std::make_unique(vertices, numVertices, indeces, numIndices))
{
}

explicit MeshImp(const char* fileName)
:
m_mesh(std::make_unique(fileName))
{
}

~MeshImp()
{
}

std::unique_ptr m_mesh;
};

typedef MeshImp const& Mesh;

class TextureImp
{
public:
friend class ResourceManager;

Don't Repeat Yourself

Consider ShaderImp, MeshImp, TextureImp. They all have the exact same structure: they befriend ResourceManager, hold onto some std::unique_ptr, which is exposable, and are privately constructible. When you see that kind of repetition in class definitions, that calls for a class template:

template 
class GenericImp
{
public:
    friend class ResourceManager;
    friend typename std::unique_ptr::deleter_type;

    std::unique_ptr const& operator->() const
    {
        return ptr_;
    }

    // might as well also provide this one
    T const& operator*() const
    {
        return *ptr_;
    }
private:
    template ::value>>
    explicit GenericImpl(U&&... u)
    : ptr_{new T(std::forward(u)...)}
    { }

    std::unique_ptr ptr_;
};

That handles all the Imps:

using ShaderImp = GenericImp;
using MeshImp = GenericImp;
using TextureImp = GenericImp;

Don't Repeat Yourself II

Now that we have our Imps, we need some maps to store them in:

template 
using Repo = std::unordered_map>;

using ShaderRepo = Repo;
...

Note that having a const char* key type is highly questionable. Using std::string prevents you from having to deal with any lifetime issues.

Don't Repeat Yourself III

Let's collapse all of our Loaders into a single function template. Because we can:

template 
static const Imp& LoadImp(Repo& map, Key&& key, Args&&... args)
{
    auto it = map.emplace(std::forward(key), 
                          std::unique_ptr(new Imp(std::forward(args)...))
                          ).first;
    return *(it->second);
}

Taking advantage of the fact that emplace() gives us where it was put in, we don't need to do the extra search. Now all the other loaders can just forward to that one, e.g.:

template 
static const ShaderImp& LoadShader(std::string const& ID, Args&&... args)
{
    return LoadImp(shaderObjects, ID, (shaderRootDir + args)...);
}

Note that (shaderRootDir + std::string(vs)).c_str() gives you a dangling pointer so you should try to avoid that construct. Prefer std::strings.

Don't Typedef Meaningless Types

When you introduces types like:

typedef TextureImp const& Texture;

That's confusing. The extra typedef adds no value. You could've just written TextureImp const&. It's not worth it. Also, you agree, since you don't actually use it anywhere yourself!

StackExchange Code Review Q#105953, answer score: 6