0

I am having troubles refactoring the following code to use proper Dependency injection. That is because I don't have access on the State Class constructors

My main limitation now is that the inject implementation mapping is done with strings and in case of a typo there will be a nice fat exception.

How can I:

  1. have compile time checks that the implementation indeed exists?
  2. having dynamic map and get rid of the strings
  3. Central point of configuration

here is some sample code to demonstrate the issue

struct IState
{
    virtual void Entry() = 0;
    virtual void Update() = 0;
};

struct ABase :IState
{
    void Entry() override { /* Default implementation..*/ }
    void Update() override { /* Default implementation..*/}
};

struct A1 : ABase
{
    void Entry() override { /*...*/ }
    void Update() override { /*...*/ }
};

struct A2 :ABase
{
    void Entry() override { /*...*/ }
};

struct BBase :IState
{
    void Entry() override { /* Default implementation..*/ }
    void Update() override { /* Default implementation..*/ }
};

struct B1 :BBase
{
    void Entry() override { /*...*/ }
};

// This is to return the desired implementation based on a key string
struct SFactory
{
    SFactory()
    {  
// This is the binding of the implementations and the States. 
// I don't really like it, 
// but I could live with it IF it was the only place 
// that the keys "A" "B" were mentioned.
        mImplementedStates.insert(std::make_pair("A", std::shared_ptr<IState>(new A2())));
        mImplementedStates.insert(std::make_pair("B", std::shared_ptr<IState>(new B1())));
    }
    static SFactory& GetInstance()
    {
        static SFactory msInstance;
        return msInstance;
    }

    std::shared_ptr<IState> GetState(std::string implementation) {
        auto it = mImplementedStates.find(implementation);
        if (it == mImplementedStates.end())
        {
            throw std::invalid_argument("Unregistered Implementation: " + implementation);
        }
        return it->second;
    }

private:
    std::map<std::string, std::shared_ptr<IState>> mImplementedStates;
};

// this is the class that I want to inject functionality. This is a wrapper of the actual implementation.
struct AStateConcrete : ThirdPartyLink
{
    // Cannot have my onw constructor because of the library
    // The library instantiate me.

private: std::shared_ptr<IState> mState;

    public: 
        // This is how I pick the  once by the 3rd party library
        void Entry()
        {
           // this is the ugly part. This "A" wont change however someone
// that wants to create a new implementation has to visit this code
// to know which "id" he should use in the factory. IF he makes a typo
// this will an throw an exception
            mState = SFactory::GetInstance().GetState("A");
            mState->Entry();
        }

        void Update()
        {
            mState->Update();
        }

        void GoB()
        {
            //...
        }
};

// this is another class that I want to inject functionality. This is a wrapper of the actual implementation.
struct BStateConcrete : ThirdPartyLink
{
    // Cannot have my onw constructor because of the library
    // The library instantiate me.

private: std::shared_ptr<IState> mState;

public:
    // This is how I pick the functionalityCalled once by the 3rd party library
    void Entry()
    {
        mState = SFactory::GetInstance().GetState("B");
        mState->Entry();
    }

    void Update()
    {
        mState->Update();
    }

    void GoA()
    {
        //...
    }
};
int main()
{
    SFactory::GetInstance();
    ThirdPartyStateMachine<ThirdPartyLink, AStateConcrete /*As initial State*/> sm; // D
    // A::Entry() is called;

    sm->Update(); // A::Update() is called (thus A2::Update();)

    sm->GoB(); 
    //  B::Entry() is called (Thus B1::Entry();)

    sm->Update(); // B::Update() is called (thus B1::Update();)
}
wxkin
  • 43
  • 1
  • 8
  • regarding my goal #1. I've found a related discussion which it might be helpful [ioc-container-check-for-errors-at-compile-time](http://stackoverflow.com/questions/9896344/ioc-container-check-for-errors-at-compile-time) – wxkin Mar 03 '17 at 12:24

1 Answers1

1
  1. having dynamic map and get rid of the strings
  2. Central point of configuration

Currently you have this mapping/relation:

                --->       --->
AStateConcrete        "A"        A2
BStateConcrete        "B"        B1

You could leave out that intermediate step and directly map:

                --->
AStateConcrete        A2
BStateConcrete        B1

For this, you could replace your map in the factory with

std::map<std::type_info, std::shared_ptr<IState>> mImplementedViews;

and use the typeid operator (which returns the needed std::type_info) to populate it.

Though this won't help you for your first point:

  1. have compile time checks that the implementation indeed exists?

For that you need to encode the information about the available implementations in some type (otherwise the compiler cannot check it). This is some metaprogramming that's a lot of fun, or you use for example boost MPL set.

Daniel Jour
  • 15,896
  • 2
  • 36
  • 63
  • Thanks for the input. I refactored as you suggested and feels already 1000 times better. However I used type_index instead of type_info as: `std::map> mStatesImplementations;` `// For a reason the std::make_pair fails. mImplementedViews.insert( std::pair>( typeid(AStateConecrete), std::shared_ptr(new A2()) ));` and in the concrete states `... mState = SFactory::GetInstance().GetState(typeid(ConecreteStateX));` – wxkin Mar 03 '17 at 08:24