Search code examples
c++dependency-injectionobject-lifetime

c++ Dependency Injection: Object lifetimes?


I'm coming from C# and trying to translate some of my practices into C++. I've used dependency injection in various places throughout my code using raw pointers. Then I decide to replace the raw pointers with std::shared_ptr's. As part of that process it was suggested that I consider using stack allocated automatic variables rather than dynamically allocating them (see this question although that question was in the context of unique_ptr so maybe that is different).

I believe the below example shows the use of automatic variables.

class MyClass
{ 
public:
   MyClass(ApplicationService& app): appService_(app)
   {
   }

   ~MyClass()
   {
        appService_.Destroy(something);

   }
private:   
   ApplicationService& appService_;
}

class ConsumerClass
{
    DoSomething()
    {
        CustomApplicationService customAppService;
        MyClass myclass(customAppService);
        myclass...
    }
}

In the above example, when customAppservice and myclass go out of scope how do I know which will be destroyed first? If customAppService is destroyed first than the MyClass destructor will fail. Is this a good reason to use shared_ptr instead in this scenario or is there a clean way around this?

UPDATE

ApplicationService is a class that is a wrapper around global functions needed to interact with a 3rd party library that my code uses. I have this class as I believe it's the standard way to support unit testing and stubbing/mocking of free standing functions. This class simply delegates calls to the corresponding global functions. The call appService_.Destroy(something); is actually destroying an object used by each specific instance of MyClass not destroying anything do with the Application class itself.


Solution

  • The answer is: you don't need to know, as your design is broken, anyway.

    First, a Destroy sounds like a bad idea, furthermore if called in an object that is not responsible for the destruction of the other object. The code from the Destroy method belongs into ApplicationService's destructor (which is hopefully virtual, although in this case it doesn't actually need to), which in contrast to C# gets called at a perfectly determined point in time.

    Once you've done this, you will (hopefully) realize, that it is not the responsibility of MyClass to destroy the appService_, as it does not own it. It is the responsibility of the ConsumerClass (or rather the DoSomething method), which really manages the actual service and which does actually destroy it automatically once you've moved Destroy's code into the destructor. Isn't it nice how RAII does make happen everything in a clean and automatic way?

    class MyClass
    { 
    public:
       MyClass(ApplicationService& app): appService_(app)
       {
       }
    
    private:   
       ApplicationService& appService_;
    }
    
    class ConsumerClass
    {
        DoSomething()
        {
            CustomApplicationService customAppService;
            MyClass myclass(customAppService);
            myclass...
        }
    }
    
    class ApplicationService
    {
    public:
        virtual ~ApplicationService()
        {
            //code from former Destroy method
        }
    }
    
    class CustomApplicationService
    {
    public:
        virtual ~CustomApplicationService()
        {
            //code from former Destroy method
        }
    }
    

    This is IMHO the perfect clean C++ way around it and the problem is definitely not a reason to spam shared_ptrs. Even if you really need a dedicated Destroy method and cannot move the code into the destructor (which I would take as a motivation for overthinking the design), then you would still call Destroy from DoSomething as again, MyClass is not responsible for destroying the appService_.

    EDIT: According to you update (and my stupid overlooking of the something argument), your design seems indeed quite correct (at least if you cannot mess with changing the ApplicationService), sorry.

    Allthough class members should get destroyed in reverse order of construction, I'm not sure this also holds for local automatic variables. What you could do to make sure the destructors get called in a defined order is introduce nested scopes using simple blocks:

    void DoSomething()
    {
        CustomApplicationService customAppService;
        {
            MyClass myclass(customAppService);
            myclass...
        }       // myclass destroyed
    }       // customAppService destroyed
    

    Of course there is still completely no need to use dynamic allocation, let aside shared_ptrs. Although the nested blocks blow the code a bit, it is nothing against the ugliness of dynamic allocation applied in a non-dynamic way and without reason and it at least "looks nice in a semantic way" with customAppService's declaration on top of the block ;)