21

I was reading about NonVirtual Interface pattern: Herb Sutter is talking about why virtual function must be private in most of the case, protected in some case and never public.

But at the end of the article he write:

Don't derive from concrete classes. Or, as Scott Meyers puts it in Item 33 of More Effective C++,[8] "Make non-leaf classes abstract." (Admittedly, it can happen in practice - in code written by someone else, of course, not by you! - and in this one case you may have to have a public virtual destructor just to accommodate what's already a poor design. Better to refactor and fix the design, though, if you can.)

But I don't understand why this is a poor design

Elvis Dukaj
  • 7,142
  • 12
  • 43
  • 85

5 Answers5

20

You could buy a copy of More Effective C++, or check your local library for a copy, and read item 33 for the full explanation. The explanation given there is that it makes your class prone to partial assignment, also known as slicing:

There are two problems here. First, the assignment operator invoked on the last line is that of the Animal class, even though the objects involved are of type Lizard. As a result, only the Animal part of liz1 will be modified. This is a partial assignment. After the assignment, liz1's Animal members have the values they got from liz2, but liz1's Lizard members remain unchanged.

The second problem is that real programmers write code like this. It's not uncommon to make assignments to objects through pointers, especially for experienced C programmers who have moved to C++. That being the case, we'd like to make the assignment behave in a more reasonable fashion. As Item 32 points out, our classes should be easy to use correctly and difficult to use incorrectly, and the classes in the hierarchy above are easy to use incorrectly.

See this question and others for a description of the object slicing problem in C++.

Item 33 also says this, later on:

Replacement of a concrete base class like Animal with an abstract base class like AbstractAnimal yields benefits far beyond simply making the behavior of operator= easier to understand. It also reduces the chances that you'll try to treat arrays polymorphically, the unpleasant consequences of which are examined in Item 3. The most significant benefit of the technique, however, occurs at the design level, because replacing concrete base classes with abstract base classes forces you to explicitly recognize the existence of useful abstractions. That is, it makes you create new abstract classes for useful concepts, even if you aren't aware of the fact that the useful concepts exist.

Community
  • 1
  • 1
rob mayoff
  • 375,296
  • 67
  • 796
  • 848
  • This is the **only** sensible answer in this post (at least as of the time of writing). – Nik Bougalis May 23 '13 at 22:52
  • Well the short answer is flexibility. But this must be the RULE so every time I want a polymorphism the base class MUST be abstract? – Elvis Dukaj May 23 '13 at 22:57
  • 1
    The suggestion is not enforced by the compiler. If it's not enforced by your employer either, than I suppose it's not a “rule”. It's up to you to decide whether you want to live by it. If you're going to be doing a substantial amount of C++ programming, following Herb Sutter's suggestions is a pretty smart idea. – rob mayoff May 23 '13 at 22:59
  • @elvis.dukaj The advice is certainly valid, but for what it's worth I don't think that this particular "rule" (at least as articulated) is that great. – Nik Bougalis May 23 '13 at 23:00
  • By the way, in a language like Java or Objective-C, where objects are *always* accessed by reference, never by value, the slicing problem doesn't exist, so most of Item 33's reasoning doesn't apply. – rob mayoff May 23 '13 at 23:01
  • @NikBougalis, I understand that you think my answer is not sensible. I will be glad to know why, since my answer applies to reference types too. (and yes, this answer is far, far better). – Elazar May 23 '13 at 23:06
  • @Elazar I think that it's sensible in that it's the only answer to provide the rationale of the **original author** of this "rule". With that said, I don't necessarily disagree with what you write in your answer. But I don't see why a class `Car` can't be concrete if you want to derive `SuperCar` from it, or how deriving from `Car` to make a `SuperCar` can break the `Car` contract. – Nik Bougalis May 23 '13 at 23:12
  • 1
    @NikBougalis say `SuperCar` can make it to 1000KPH, where everything inside `Car` (or client code expecting `Car`) assumes it will not be a sensible value - it is a concrete car, after all - and used it accordingly. This restriction will be implicit, probably falling out of `Car`'s implementation. My answer does not apply if the contract is explicit, correct and full; but from what I have read, this is rarely the case. – Elazar May 23 '13 at 23:20
  • @Elazar that is a fair point, and that's part of my "don't necessarily disagree" portion of your answer. – Nik Bougalis May 23 '13 at 23:24
6

In general, it's difficult to strictly maintain a contract between two distinct concrete objects.

Inheritance becomes really easier and more robust when we deal with generic behavior.

Imagine that we want to create a class named Ferrari and a subclass named SuperFerrari. Contract is: turnTheKey(), goFast(), skid()

At first glance, sounds like very similar class, with no conflict. Let's go with inheritance of these both concrete classes.

However, now we want add a feature to SuperFerrari: turnOnBluRayDisc().

Problem: Inheritance expects a IS-A relationship between components. With this new feature, SuperFerrari is not a simple Ferrari any more with its own behavior; it now ADDS behavior . It would lead to some ugly code, with some cast needed, in order to choose the new feature while dealing with a Ferrari reference initially (polymorphism).

With an abstract/interface class in common to both classes, this problem would disappear since Ferrari and SuperFerrari would be two leafs, and it's more logic since now Ferrari and SuperFerrari should not be consider as similar (not a IS-A relationship any more).

In short, derive from concrete class would lead in many cases to a poor/ugly code, less flexible, and hard to read and maintain.

Making a hierarchy driven by abstract class/interface allow concrete children classes to evolve separately without any issues, especially when you implement some subhierarchies dedicated to a special amount of concrete classes without boring others leafs, and still benefit from polymorphism.

Mik378
  • 21,881
  • 15
  • 82
  • 180
  • 1
    I really don't follow... a `SuperFerrari` is every bit a `Ferrari` - it just has a fancy Blu-Ray player. So wwhy can't it derive from `Ferrari` and why is having some sort of `FerrariCar` base class that both `Ferrari` and `SuperFerrari` derive from better? What benefit, if any, does it get us? – Nik Bougalis May 23 '13 at 22:50
  • @Nik Bougalis When having a datastructure containing some Ferrari's, one would iterate over, check for the effective type and turn the bluray disc if it's a `SuperFerrari`. IMO, this reduce the elegancy of polymorphism. Better, let's inverse the sample. Each new feature added to a `Ferrari` would be part of the `SuperFerrari`, even whether it does make no sense at all. No way to evolve with new features separately. Both concrete classes must not be part of the same branch. – Mik378 May 23 '13 at 23:02
  • 1
    I don't absolutly agree. Base class is base because it implements only the common interface. Derived class has to implement new features. – Elvis Dukaj May 23 '13 at 23:07
  • @Mik378 A common base class wouldn't do anything to solve the "iterate list and turn on bluray disc in those models that have it" problem you describe unless it implemented a function to do so, that by default did nothing. If it doesn't make sense for every `Ferrari` feature to be in the `SuperFerrari` then yes, you need a base class. But what if it does? Why **impose** the need for a base class when the `Ferrari` will do just fine. No, sorry. I just don't buy your explanation. – Nik Bougalis May 23 '13 at 23:07
  • @elvis.dukaj What you wrote make me think to that: (from Effective C++) "Anytime you find yourself writing code of the form "if the object is of type T1, then do something, but if it's of type T2, then do something else," slap yourself". When you ADD features to derived class from a concrete class, you are typically in this case and reduce the concept of OO with some procedural boilerplate. – Mik378 May 23 '13 at 23:15
  • @Nik Bougalis I agree that the sample is not meaningfull but IMO it describes the concept. It's just an advice (that I follow), that really reduce the risk of: Liskov principle violation, interface segregation violation, and dealing with some concrete classes as reference type directly increases coupling between components and also make unit tests really harder. – Mik378 May 23 '13 at 23:20
  • @Mik378 In your example Ferrari and SuperFerrari has a common interface and a SuperFerrari is a Ferrari. But Ferrari can be considerated a concrete class. But this is not a good design. Is better have FerrariAbstract to the leaf abstract and Ferrari deriving from FerrariAbstract and SuperFerrari deriving from Ferrari. This seems strange to me. I wonder know why this is the right design – Elvis Dukaj May 23 '13 at 23:21
  • @elvis.dukaj My broken design in my answer was: `SuperFerrari` extends `Ferrari`. (both concrete) My solution was: `Ferrari` extends `AbstractFerrari` and `SuperFerrari` extends `AbstractFerrari`. `SuperFerrari` extends `Ferrari` is a an error of design that would lead to procedural code, Liskov violation and Interface Segregation violation. – Mik378 May 23 '13 at 23:26
  • 1
    Still, I don't buy it. Unless a `SuperFerrari` is not *always* a `Ferrari` a new abstract base for both is just extra fluff. – Nik Bougalis May 23 '13 at 23:32
  • @Nik Bougalis I agree. Of course, if `SuperFerrari` does not bring something new, an extra base class would be unnecessary. But in my sample, I expect them to be different since the latter add a feature. Classes blending `SuperFerrari` and `Ferrari` usage would involve a serie of if/else to check for the effective type. Typically breaking the Open/Closed principle, essence of POO. My rule is, if it's obvious that, with time, related concrete classes will differ with different features, break the relationship. – Mik378 May 23 '13 at 23:37
  • @Mik378 I can agree with that. – Nik Bougalis May 23 '13 at 23:41
5

I think this is because concrete class has concrete behavior. When deriving from it, you are committed to retain the same "contract", but the actual contract is defined by the specific implementation of the base class, and in reality will have many subtleties you will probably break without knowing.

Disclaimer: I am not an experienced developer; it is just a suggestion.

Elazar
  • 20,415
  • 4
  • 46
  • 67
  • 1
    It is often the case in some other languages that classes are designed both to be concrete **and** to be subclassed in specific ways, with documentation describing those ways. (`UIView` in the iOS Objective-C SDK is a good example.) There are specific properties of C++ that make this practice dangerous. – rob mayoff May 23 '13 at 23:15
5

A problem with inheriting from a concrete type is that it creates some ambiguity as to whether code which specifies a certain type really wants an object of the specific concrete type, or wants an object of a type which behaves in the fashion that the concrete type behaves. This distinction is vital in C++, since there are many cases where operations which will work correctly on objects of a certain type will fail badly on objects of derived types. In Java and .NET, there are many fewer situations where one can use an object of a particular type, but could not use an object of a derived type. As such, inheriting from a concrete type isn't nearly as problematical. Even there, however, having sealed concrete classes which inherit from abstract classes, and using the abstract class types everywhere except in constructor invocations (which must use the concrete types) will make it easier to change parts of the class hierarchy without breaking code.

supercat
  • 77,689
  • 9
  • 166
  • 211
0

When you derive from a concrete base class you inherit functionality, which may not be revealed to you if you do not have the source code. When you program to an interface (or abstract class) you don't inherit functionality meaning it it a better way to do things like expose an API. That being said I think that there are plenty of times when inheriting from a concrete class is acceptable

aaronman
  • 18,343
  • 7
  • 63
  • 78