1

I found the below code in our code base at work, and I think it looks strange. The class RetainCounting is used for implementing reference counting.

1.Should the IProvider really derive from RetainCounting? Isn´t it better if it was just an abstract base class and then let the class that implements IProvider i.e. ProviderImpl derive from the "RetainCounting" instead (i.e. if it needs retain/release features)?

2.If we believe that IProvider should derive from RetainCounting, why do ProviderImpl needs to derive from "RetainCounting" as well, when the IProvider already has this inheritance?

class RetainCounting
{
public:
   virtual ~RetainCounting();
   void retain();
   void release();

private:
   // more code....
};

class IProvider : public virtual RetainCounting
{
public:
   virtual void doSomething() = 0;
};

class ProviderImpl : virtual private IProvider,
                     virtual public RetainCounting
{
public:
   void doSomething() override;
};
0xBADF00
  • 1,028
  • 12
  • 26
  • Well, IProvider is certainly not a pure interface if it does inherit something that's not a pure interface. – eerorika Oct 09 '15 at 07:28
  • Yes you are right, but is it a good design decision to have IProvider inherite from RetainCounting, I do not see the point of that? I will modify my question. – 0xBADF00 Oct 09 '15 at 07:32
  • here is the good explanation about inheritance and virtual keyword http://stackoverflow.com/questions/21558/in-c-what-is-a-virtual-base-class – vishal Oct 09 '15 at 07:32
  • ProviderImpl only needs to derive from IProvider, as IProvider already derives from RetainCounting. –  Oct 09 '15 at 07:33
  • This design looks like stratified design. There once was one initial design decision (one of the 2 you presented), and later a need that could not be easily fullfilled so someone decided to change it but only changed the minimum. Sadly enough this is common when code ages :-( – Serge Ballesta Oct 09 '15 at 08:52

2 Answers2

1

2.If we believe that IProvider should derive from RetainCounting, why do ProviderImpl needs to derive from "RetainCounting" as well, when the IProvider already has this inheritance?

If it must inherit RetainCounting publicly, then yes it must inherit it because IProvider - and transitively it's own RetainCounting base - is inherited privately. But you may inherit IProvider non-virutally unless something inherits multiple IProvider derivatives.

If there is no need for the visibility difference, then you can simplify the design by getting rid of the virtual inheritance, and only inheriting IProvider.

class IProvider : public RetainCounting

class ProviderImpl : public IProvider // or private, depending on what you need

1.Should the IProvider really derive from RetainCounting? Isn´t it better if it was just an abstract base class and then let the class that implements IProvider i.e. ProviderImpl derive from the "RetainCounting" instead (i.e. if it needs retain/release features)?

If all derivatives of IProvider don't need to be reference counted, then IProvider shouldn't inherit RetainCounting and that should only be done in the derivative instead.

Interface usually means a class that has only pure virtual functions and no member variables. If IProvider inherits something that is not an interface, then it's not an interface itself.

If all defivatives of IProvider must be reference counted but you want IProvider to be an interface such as is usually implied with I-prefixed name, then what you could do is define an interface for reference counting. Let's call it IRetainCounting. Since the implementation is probably shared by many derivatives, you can create a base class that implements it. This causes you to need virtual inheritance.

class IRetainCounting
{
public:
    virtual ~IRetainCounting(){}
    virtual void retain() = 0;
    virtual void release() = 0;
};

class RetainCounting: public virtual IRetainCounting
{
public:
    void retain() override;
    void release() override;

private:
   // more code....
};

class IProvider : public virtual IRetainCounting
{
public:
    virtual void doSomething() = 0;
};

// option 1
class ProviderImpl : private IProvider,
                     public RetainCounting
{
public:
    void doSomething() override;
};

// option 2, you can keep BaseRetainCounting private if you want to
// I recommend this if there can be more than 1 implementation of IRetainCounting
// otherwise 1 is simpler
class ProviderImpl : private IProvider,
                     private RetainCounting,
                     public virtual IRetainCounting

The advantage: pure interfaces

The disadvantage: more code (but only a constant amount. The amount of boilerplate does not scale up with the number of derivatives.) and the need for virtual inheritance (which you need anyway for the visibility difference)

eerorika
  • 232,697
  • 12
  • 197
  • 326
0

Should the IProvider really derive from RetainCounting? It is just a pure interface, shouldn´t the class that implements IProvider i.e. ProviderImpl derive from the "RetainCounting" instead?

Well, if you want IProvider to have retain() and release(), so yes, you could derive it from RetainCounting. That another derived class already has RetainCounting does not matter. So to summarize, the question is: Do IProvider need retain() and release() features?

2.If we believe that IProvider should derive from RetainCounting, why do ProviderImpl needs to derive from "RetainCounting" as well, when the IProvider already has this inheritance?

That IMO does not make sense in this specific case. You could easily carry this kind of double inheritance with diamond-like classes, but in this specific case I do not see any benefit.

Adrian Maire
  • 14,354
  • 9
  • 45
  • 85
  • For your first comment, I would say that if the class that implements IProvider needs retain() and release() features, it should derive from RetainCounting it self. Adding this kind of inheritance to an abstract class makes no sense to me? – 0xBADF00 Oct 09 '15 at 07:38