ANYbotics/kindr

Why use private inheritance rather than composition?

Closed this issue · 4 comments

This design seems strange to me:

https://github.com/ethz-asl/kindr/blob/master/include/kindr/poses/eigen/HomogeneousTransformation.hpp#L45-L46

The transformation is built with private inheritance rather than composition. What is the rationale behind this design choice?

First of all this is a implementation detail. And does not need to be pondered that much. There are some advantages and some drawbacks of that. But it is quite common as an alternative to members.

Especially one non obvious (performance / memory conservative) reason, I know of:
It is the (only) possibility to relax the requirement for any object (including member objects) having at least the size of 1 byte in memory (which hold even for objects of empty structs/classes!). So base objects are allowed to have zero size! (see for example http://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Empty_Base_Optimization)
This for example is - as far as I know the reason why std::tuple is typically (implementation detail again) having its data not as members but as private bases. This is especially useful if the candidate class (for member or private base) are dependent names and might be even empty. Imagine for this case a implementation that is either rotation only or translation only. The other component will then not require any space but will waste usually effectively word size as a member (!, because of alignment rules in the layout of objects) in comparison to as a private base class.

The biggest drawback I'm aware of is the fact that the effective inheritance gives rise to the implicit convertibility for reference types to references to these private bases but nobody is allowed to actually perform these conversion (except the class and its friends of course). That can have bad effect on overloaded functions (might even make the overloading ambiguous - which is an error!). Overload resolution does in general completely ignore private modifiers. And ambiguity in overload resolution can not be solved by making something private (sad but true)!

Apart from arguments like these it is of course a matter of taste and comfort. For example members of private bases are of course easier to address than members of members...

Unfortunately the ambiguity of the multiplication operator is quite likely to happen with the current implementation - of course depending on where the operator is defined. But we can wait till it actually happens - as it is an implementation detail and allowed to change at any time. If we then still want the empty base optimization possible (at least for one of the two) we can still have a special member being a child of both (e.g. a std::pair). Then we have the benefit of both concepts. Only the pair of identity rotation and identity translation will then still waste memory.

The ambiguity of multiplication is what led me to this problem. I hit this immediately when working on these extensions. As this is a case that actually comes up during implementation, can we see the private inheritance use here as an instance of early optimization?

Also, I don't see it as an implementation detail that shouldn't be pondered. If we hope for a wide group of people to use and maintain this code, we have to worry about whether or not the implementation is weird or hard to understand. My understanding is that private inheritance should only be used (a) as an optimization as you describe, or (b) if you need access to private members. In this situation we should not access private members. So, this is something we should think about.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Inheritance

http://en.wikipedia.org/wiki/Composition_over_inheritance

I agree, it could be seen as an case of early optimization. And I'm sorry I I indirectly introduced that.

And if you already got this ambiguity then let us got for members instead. That clearly could be a simplification increasing readability.

But I don't agree that this is really a problem of composition over inheritance as private bases, which do not even exist in the pure OO idea, are rather a case of composition than inheritance from the point of view of that discussion because you do not publish the inheritance.
That means you still have to delegate the outside world to the base objects - just like private members. That is all you need. The problem of public inheritance is that it becomes a part of the public API and user rely on it (cannot happen in private inheritance). That can make the code much less flexible then in the composition case.

We keep the inheritance in kindr 1.0.0 due to limited work force.