Gfrktyl

So I'm working on a game framework and one of the things I need are access to stateful "services" that can be accessed from all over. I was initially going to use the Singleton pattern but decided to try and come up with alternatives to avoid the short comings of that pattern.

So here is what I've got so far.

Main.cpp

#include "Engine.h"

#include "Logger.h"



int main(int argc, char* argv) {

    Engine engine;



    // old

    {

        auto logger = new Logger();



        engine.provide(logger);

        engine.getLoggerOld().log("old TEST");



        engine.provide(nullptr);

        delete logger;

    }

    engine.getLoggerOld().log("old TEST");//wont print, just calls empty funcs

    // old





    // new

    {

        auto splogger = std::make_shared<Logger>();



        engine.provide(splogger);

        engine.getLoggerNew()->log("new TEST");

    }

    engine.getLoggerNew()->log("new TEST");//wont print, just calls empty funcs

    // new



    return 0;

}

Engine.h

#pragma once



#include <memory>



#include "ILogger.h"

#include "NullLogger.h"



class Engine {

public:

    Engine() {

        //old 

        serviceLogger = &nullLogger;



        //new

        spLoggerService = std::make_shared<NullLogger>();

    }



///////// old

public:

    void provide(ILogger* service) {

        if (service) {

            serviceLogger = service;

        } else {

            serviceLogger = &nullLogger;

        }

    }



    ILogger& getLoggerOld() const {

        return *serviceLogger;

    }

private:

    ILogger* serviceLogger;

    NullLogger nullLogger;



///////// old



///////// new

public:

    void provide(std::shared_ptr<ILogger> loggerService) {

        wpLoggerService = loggerService;

    }



    std::shared_ptr<ILogger> getLoggerNew() {

        if (wpLoggerService.expired()) {

            wpLoggerService = spLoggerService;

        }

        return wpLoggerService.lock();

    }

private:

    std::shared_ptr<ILogger> spLoggerService;

    std::weak_ptr<ILogger> wpLoggerService;



///////// new

};

I'm a bit unsure on how I should implement the services, in the above example(s) "Engine"(The Service Locator) essentially is designed to never really "own" the services, and instead it only holds a reference to the services, it's then the externally determined if/how/when the services become invalid.

In the example sections labeled "old", these are basically how I saw this pattern implemented elsewhere. I don't like that its left up to how its used to determine if the pointers will be valid or not.

In the example sections labeled "new" I tried to implement the more modern smart pointers, I like the automatic cleanup of the expired references, etc. But I'm still not sure if I like the way the references are handled.

So I guess my questions are:

1. Does it make sense to give total ownership of the service to the Engine(locator) and allow its life cycle to manage cleanup?


2. Should I stay with the raw pointer "old" way I found in online examples?


3. Should I keep going with the "new" smart pointer way?(one thing I'm not a fan of here is needing the if (wpLoggerService.expired()) check as well as the .lock() when accessing a service.(maybe there is a better smart pointer way to avoide this?)

Also if it makes sense for the Engine(Locator) to manage the life-cycle of the services, I was thinking of maybe making the "provide" function similar to that of the std::vector's emplace_back where you would do something like: engine.provide<Logger>(/*ctor args*/) and the .provide<T>() function would instantiate the object internally to prevent any ownership/access to it externally. (This just seems like a more reasonable way to let the Engine control its life-cycle, or is this dumb?)

Disclaimer: New to C++, first time using smart pointers, please be gentle :3

Note that the "old" version should store the logger in a std::unique_ptr in main, rather than explicitly calling new and delete.

The simplest possible thing would be something like this:

struct Services // previously `Engine`

{

    Logger logger;

};



int main()

{

    {

        Services services;

        services.logger.log("yep");

    }



    //services.logger.log("nope"); // obviously doesn't compile

}

To use the logger "somewhere else" we can pass it into an object constructor by reference, (or pass the entire Services object by reference):

struct Object

{

    explicit Object(Logger& logger):

        logger(&logger) { }



private:



    Logger* logger;

};

If necessary, the object can store the logger (or Services) by reference (which has the side-effect of preventing us defining assignment operators for Object as we can't re-assign a reference), or by pointer (which allows us to define assignment operators, but can also be reassigned to a nullptr (for better or worse)).

Either way, we still depend on the lifetime of the Logger being longer than the lifetime of the Object. With C++, this is usually enforced simply by the implicit order of destruction of stack objects (and any child objects they contain), e.g.:

{

    Services services;

    Object object(services.logger);



} // destruction in reverse order: first object, then services.

It might seem like we can use std::shared_ptr and std::weak_ptr to enforce the above constraint (longer service lifetime), but it adds overhead and complexity, and can lead to more serious issues. When we promote a weak_ptr to a shared_ptr, the new shared_ptr also "owns" the service. So the lifetime is no longer defined by Services, but by the service user as well.

If we enforce the Object lifetime constraint properly ourselves, we don't need this. If we don't enforce that constraint, then the exact moment of a service being destroyed becomes very difficult to determine, which leads to similar problems as with Singleton objects.

This article has some more arguments against using std::shared_ptr, and points out that what we actually need is some sort of checked pointer or borrowed reference for debugging. Unfortunately, this doesn't exist in the C++ standard library.

Note that if it's actually necessary to swap out, or destroy a service part-way through use, we can't store any pointer or reference to the service. It must be fetched from Services every time it's accessed. This is probably quite difficult to enforce though.

In short:

• Avoid using this pattern if possible.


• Pass dependencies by reference in constructors (or other functions).


• Ensure that lifetimes are well-defined and easy to understand (i.e. avoid std::shared_ptr).


• Ensure that the lifetime of the Engine / Services exceeds the lifetime of anything that uses them.


• Don't move or copy the individual services, or the Services class.

And to answer your questions:

1. Yes.


2. Allowing direct access to the service member, returning a raw pointer from an accessor function, or returning a reference from an accessor function are all reasonable options. The first is simplest unless you have some other constraints (enforcing access through an interface, or enforcing const correctness).


3. I'd argue not (see above).