HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Using ID's with a "scope" -like hierarchy

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withhierarchylikescopeusing

Problem

I currently have a ResourceManager class which is responsible for searching for resources with a given identifier, returning a reference counted pointer to the resource if a valid resource is found, and instantiating a new object if none has been found. Often a resource itself depends on other resources, which are requested from the resource manager. For example, Window may depend on SdlMain (which initializes and shuts down SDL), and thus the constructor of Window would ask ResourceManager for the SdlMain object.

Example:

ResourceManager resources;
//the return type is std::shared_ptr, the first argument "MainWindow" is the 
//..ID to search for, the rest of the arguments are simply forwarded to the constructor 
//..of class Window if needed.
auto mainWindow = resources.make("MainWindow", "Hello World",
                    512, 512, SDL_WINDOW_SHOWN, SDL_RENDERER_ACCELERATED |
                    SDL_RENDERER_PRESENTVSYNC);
//The Window object contains the renderer, too, so the ID of the current window 
//"MainWindow" must be passed to the texture's constructor
auto tex1 = resources.make("tex/tex1.png", "tex/tex1.png", "MainWindow");
//This renders the texture to the screen.
tex1->renderToBuffer(0,0);
mainWindow->bufferToDisplay();


However, the requirement of knowing the actual ID of the Window when creating a texture becomes a problem when writing a sprite class, if multiple window objects are to be supported:

//Unlike Texture, a Sprite is not a resource but it's lifetime is bound to the  object
//..owning it (and there may be many sprites using the same Texture). Thanks to 
//..how Texture is implemented, now everything that wishes to use a sprite must be 
//..aware of the ID of the window "MainWindow" in order to initialize a Sprite.
Sprite sprite(resources, "tex/tex1.png", "MainWindow");


I recently came up a solution for this. I would give the identifiers a context, similar to the "scope" of programming languages. SdlMain would be

Solution

Preface: these are just my personal opinions based on experience.

Don't force the user of the class to remember to do stuff

The setContext(const char*) method is a method which the user of the class is required to call to have proper operation. As such the user is at risk of forgetting to call this method and thus introduce a bug in the software. Further more the user must remember to be consistent in spelling of the context name or face the risk of introducing subtle bugs where different contexts are used by mistake.

If you wish to go with the concept of contexts (say that 10 time quickly), I would make a "context" a first class object which is required for creating instances of resources. Like this:

class ResourceContext{
public:
    template
    std::shared_ptr make(Arg... args);
};

class ResourceManager{
public:
    std::shared_ptr createContext(const char*);
    std::shared_ptr findContext(const char*);
private:
    friend class ResourceContext;
};

ResourceManager mgr;
auto gameContext = mgr.createContext("Game");
Sprite sprite(gameContext, "tex/tex1.png");


This approach solves my two worries

-
You can not forget to change context. You are forced to find the correct context.

-
You can not silently introduce a typo, either you explicitly create a new context or you look up an existing one (with an error if it doesn't exist).

Lookup by reference when possible

If your texture/sprite class is dependent on the window they are displayed in (as opposed to the SDL context) then it is quite reasonable to expect them to be created from a context where the window is available. Why not simply pass an instance of the resource to the constructor of the sprite/texture instead of doing a lookup by string ID?

Your current approach implicitly reserves "magic" names for specific instances of specific classes the risk here is that an unknowing user may create some kind of conflict because they didn't know of all the reserved names in your system.

One size doesn't fit all (Stop sugar coating my globals)

In general I believe that this kind of resource manager should only be used for obtaining external resources such as textures, sounds etc. To me it looks like you're trying to sugar coat having global variables, because that's essentially what this is being used like in your example. Using it for SDL context is overkill, why would you want to shutdown SDL during any point of your program's lifetime? What is the point of having it managed as a resource over just a simple global?

In all honesty, I think you are over-engineering this.

Code Snippets

class ResourceContext{
public:
    template<typename... Arg>
    std::shared_ptr<Resource> make(Arg... args);
};

class ResourceManager{
public:
    std::shared_ptr<ResourceContext> createContext(const char*);
    std::shared_ptr<ResourceContext> findContext(const char*);
private:
    friend class ResourceContext;
};


ResourceManager mgr;
auto gameContext = mgr.createContext("Game");
Sprite sprite(gameContext, "tex/tex1.png");

Context

StackExchange Code Review Q#41989, answer score: 5

Revisions (0)

No revisions yet.