0

My concern is that when using either shared_ptr or unique_ptr I stick to one ownership model - either injected objects is shared or my own. And I think this is is secondary class responsibility - to care of injected objects lifetime.

So, does it violates SRP - assuming that class already has some responsibility.

Some simple example:

  class Calculator {
  public:
      Calculator(std::unique_ptr<Adder> adder) : adder(adder) {}
      void add();
  private:
      std::unique_ptr<Adder> adder;
  };

When design changes - so I will have many different calculators - then I need to change unique_ptr to shared_ptr. So even if Calculator main responsibility (to calculate) did not change - I need to change the class.

Wouldn't be better to use simple references for injected objects - and just left the responsibility of injected objects lifetime to some other classes?

PiotrNycz
  • 23,099
  • 7
  • 66
  • 112
  • 'change the class' literally yes, but you wouldn't be changing any of the *functionality* of the class so for that reason alone one could argue you're actually not changing the class. – stijn Mar 11 '16 at 14:30
  • Contructors aren't considered part of the interface, I'm not sure what the problem with many different calculators with different dependancies is? – Nathan Cooper Mar 11 '16 at 14:31
  • @stijn But I need to change unit tests as well - e.g. injecting AdderMock by either of smart pointers. Anyway - I put this question because I am not sure... – PiotrNycz Mar 11 '16 at 14:32
  • Note that DI doesn't mean 'inject one single instance everywhere', so another question to ask yourself is whether it's really needed to use a shared Adder instance instead of multiple unique instances? And yes if you make changes to a class you need to change tests. That doesn't mean you are violating SRP though. – stijn Mar 11 '16 at 14:34
  • @stijn - this is only example. Assume that `Adder` needs to be shared. The real project is not about `Calculator` nor `Adder` - it is much more complicated and we have doubts if using smart pointers is not additional "reason to change" when have to frequently change this smart pointer types. – PiotrNycz Mar 11 '16 at 14:37
  • If you're 'frequently' changing smart pointers you should head back to the design table and rethink the design to start with.. But maybe add a more concrete explanation of what your actual classes do to the question, that makes things more clear for us. – stijn Mar 11 '16 at 14:38
  • WRT DI, you should have a conceptual model of which component depends on which other component. No crosslinks are allowed. Any downlink across components must be a shared pointer, any uplink must be a weak pointer to a callback type (and checked on each use). – dascandy Mar 11 '16 at 14:39
  • @stijn - sure - but I really only want an answer to my simple question... – PiotrNycz Mar 11 '16 at 14:39
  • @dascandy - is your point that instead of having everywhere references - I should rather have `shared_ptr`? We had so - but that was too big memory overhead in our case. – PiotrNycz Mar 11 '16 at 14:41
  • The things you share should be substantial enough that the shared_ptr overhead isn't a big deal. If they can reasonably not share the instance, they should by default not be sharing it. – dascandy Mar 11 '16 at 14:42

1 Answers1

0

No, the way we kept member variables in an object is the implementation detail, and it is not related in any way to design principles like Single Responsibility Principle

To illustrate this - you can encapsulate access to your members by some private method - this prevents the changes in class implementation when you changes from unique_ptr to shared_ptr or vice versa.


private:
   Adder& add();
   Adder const& add();

Or you can go even further and enclose Adder to some private object - thus preventing it totally from accidental access to "read" adder variable, like:

class Calculator 
{
   class AdderProxy
   {
   public:
       using Type = std::unique_ptr<>;
       AdderProxy(AdderType);
       void add(...);    
   };
public:
   Calculator(AdderProxy::Type);

private:
   AdderProxy adder;

Or you can just have some DI library, like this one - and have all kind of injection hidden from application code.

PiotrNycz
  • 23,099
  • 7
  • 66
  • 112