[Discussion] Modifying a class' behaviour - inheritance pitfalls

[D_Drmmr]

Interesting, but I don't see the relevance as it talks about derived classes changing the behaviour. That article however shows why I dislike non-private data members.

If the classes in your example have non-virtual assignment operators, then the code below would change the contents of the string without the assignment operator of the derived class being called.

void foo(String& s)
{
    s = String("foo");
}

void bar()
{
    FastString s("foobar");
    foo(s);
    // uh oh, s.length() is incorrect
}

For this example to work correctly, the base class' assignment operator has to be virtual and the derived class needs an overloaded assignment operator taking a base class object (to override the virtual function in the base class).

[monarch_dodra]

Well, before we enter a flame war, the big problem is that a lot of classes have nothing more than the name of the functions in their interfaces as documentation. So you really have no choice other than trying to guess what the class does. Sure, avoid doing so if you can, but if there are no alternatives, it's the least evil of solutions.

That said, I would say that the lack of any function that can change the string length imply the string length's is necessarily constant, is a matter of discussion.

IMO, unless the class' name is "ConstLengthString", I would assume that nobody has yet had the need to change the length of the string, and as such, the function "resize" is not included... yet.

[GCDEF]

I see your point, but I think your example is somewhat contrived. If the length in the base class never changes, there'd be no need to make the length method virtual, so that scenario is unlikely to exist in real life.

Also, overriding base class behavior in a derived class and depending on a particular implementation of the base class, is definitely bad design and implementation. The base class implements length and it should be expected that there's a chance your overriden method will break if the base class changes.

So, while your scenario is possible, it requires both bad class design and bad programming practices to occur.

[Zaccheus]

Yes my example is rather contrived. I came upon the issue while investigating something else.

Again, the implementation of the base class is irrelevant to the derived class. It is completely hidden. The base class could be changed to using a vector and the derived class would still be ok.


The question I was really interested in was how the base class author can best update his class without causing problems for existing derived classes. One thing we all agreed on was for the base class to be properly documented.

I'm still wondering about whether the base class could have been written in such a way that another derived class (MutableString) could have been written via protected functions, but that would have made that derived class dependent on the base class' implementation, something we all agree is a bad thing.

D_Drmmr, thanks for that assigment example.

[Lindley]

I think the basic flaw in the reasoning here is that there's no good reason for the base class to make that particular method virtual in the first place. The notion of length is too tightly bound to the particular implementation to be something worth overriding.

[GCDEF]

I think the basic flaw in the reasoning here is that there's no good reason for the base class to make that particular method virtual in the first place. The notion of length is too tightly bound to the particular implementation to be something worth overriding.

Exactly. That's just asking for trouble. If a change to a base class can break a derived class, there's a design flaw. In this case the string is owned by the base class and its operations should be maintained by the base class, specifically to avoid this kind of problem.

[superbonzo]

example contrivedness apart, my point is that in a language like C++ where you have to respect guarantees, pre/post conditions all the time in order to write a correct program the public interface as-it-is (eg. without further documentation) is nearly useless ( excluding trivial or self-evident cases, of course ). And every deduction that is merely inferred from members naming alone is at least risky. So, if, as suggested by monarch_dodra, the class String were named ConstLengthString then we could reasonably conclude that the string size is and will always be constant. Otherwise, the best one could do is to guess that the string was implemented as a constant size string, which IMHO is invoking an implementation detail.

For example, if one substitutes the private member "char* data" with a "std::string data" and implements the member length() and c_str() through the corresponding std::string members then your derived class will break because strlen( c_str() ) would not always be equal to length() in general. The only way for avoiding such situation is to specify at the documentation level that "strlen( c_str() ) == length() " always holds true or not.

[superbonzo]

the big problem is that a lot of classes have nothing more than the name of the functions in their interfaces as documentation. So you really have no choice other than trying to guess what the class does. Sure, avoid doing so if you can, but if there are no alternatives, it's the least evil of solutions.

so, you are saying that in professional teams you use classes with undocumented funcionalities ? I ask because I find that quite surprising ... ( being a non-professional coder myself )

[monarch_dodra]

so, you are saying that in professional teams you use classes with undocumented funcionalities ? I ask because I find that quite surprising ... ( being a non-professional coder myself )

Yes. You have to use code written and owned by others, that never bothered documenting it. You have no choice.

But keep in mind Good/Passionate coder != Professional coder.

...unfortunately...

[Zaccheus]

I think the basic flaw in the reasoning here is that there's no good reason for the base class to make that particular method virtual in the first place.

Yes absolutely, I agree. Although it was just meant to be a simple example, you make a good point that the base class author needs to be careful what behaviour the derived classes can change.

For example, if one substitutes the private member "char* data" with a "std::string data" and implements the member length() and c_str() through the corresponding std::string members then your derived class will break because strlen( c_str() ) would not always be equal to length() in general.

Eh?

[laserlight]

Eh?

The string object may contain null characters in the middle.

[Zaccheus]

The String constructor only takes a null terminated const char*, making it impossible for that to happen.

[superbonzo]

The String constructor only takes a null terminated const char*, making it impossible for that to happen.

well, I thought this thread were about innocuous behavior extensions of the base class that breaks the derived class behavior ... exactly in the same way you're worried by a non constant string size extension you should be worryed by a seemingly innocuous String(const std::string&) constructor, aren't you ? indeed, I think that a programmer accustomed to std::string that sees undocumented c_str() and length() members would at least be surprised knowing that strlen(c_str()) == length() always holds true, especially if an std::string ctor is provided.

[JohnW@Wessex]

Unless you only allow inheritance from purely abstract interface classes or maybe adding the Java style 'final' to the language, then this will always be a potential problem. I view the 'only inherit from abstract interface' rule to be overly restrictive and to much along the lines of Java's 'you may hurt yourself using this technique, so nobody is allowed to use it' philosophy, for me.

Part of my remit in my current job is setting coding standards and I am the main contributor and architect of our internal class library.
No one can ever really guard against persistent laziness and stupidity, but instilling a sense of 'doing the right thing' in the department (and management) goes a long way to ensuring that nasty surprises are kept to a minimum. Part of this is ensuring that documentation, such as that created by doxygen, is always a part of the coding process and not an afterthought that someone will get around to doing one day (i.e. never).
Classes in our library document which methods are expected to be overridden if used as a base class, the more complex ones come with examples of use. Implementation details are kept as private (or protected) as possible, to achieve the necessary functionality.

If you allow your code base to become opaque through the lack of even basic documentation you will almost certainly end up with a collection of repeated, inconsistent and sometimes incompatible code.

I speak from experience, not just of my code, but from working in teams where 'Not Invented Here' was the apparent mantra.

[Zaccheus]

well, I thought this thread were about innocuous behavior extensions of the base class that breaks the derived class behavior ... exactly in the same way you're worried by a non constant string size extension you should be worryed by a seemingly innocuous String(const std::string&) constructor, aren't you

Oh I agree, that would cause problems and yes that is exactly the kind of changes I was talking about - changing the public interface of the class, not merely a private implementation detail.