[Discussion] Modifying a class' behaviour - inheritance pitfalls

Let's say we had the following class. As it stands, once constructed there is no way to change the contents of the string.

Code:

---

class String
{

String(const char* text);
{
    data = (char*)malloc(strlen(text)+1);
    strcpy(data, text);
}

virtual size_t length() const
{
    return strlen(data);
}

const char* c_str() const
{
    return data;
}

// destructor, copy and assignment implemented.
// ...

private:
    char* data;
};

---

Now someone might extend that class to report the length faster. We know the length never changes so we only need to measure it once:

Code:

---

class FastString : public String
{
public:
    FastString(const char* text) : String(text)
    {
        len = strlen( c_str() );
    }

    virtual size_t length() const
    {
        return len;
    }

// destructor, copy and assignment implemented.
// ...

private:
    size_t len;
};

---

However, if the original class 'String' is then changed such that its contents can be changed, the derived class will stop working. Note that this would happen despite the implementation of the original class being completely private.

An alternative would be that the original class is never changed, but that the new behaviour is also implemented in a separate derived class. Downside being that the original class would have to expose some of its implementation to derived classes, maybe by making data protected rather than private. That in itself opens up even more problems because then even the implementation cannot be changed without breaking derived classes.

The only solution I see is documenting what is guaranteed about a class and other authors not making assumptions about how the base class will behave in future.

The documentation might say that String will always remain immutable, in which case FastString should always work, but if no such guarantee is given then the author of FastString should be very careful about the assumptions he makes.

Deriving from something that isn't meant to be derived from should always be done with extreme care. The only time I do it is to overcome the lack of C++0x template aliases in current compilers.

What about making Fast_String contain String instead, even going so far as to use pimpl ?
All changes would then be via calls to Fast_String and thus len could be maintained.
edit: not strictly answering an inheritance question i know :)

Yes, that would have been a solution if the author of FastString had been aware that the String behaviour might change.

Quote:

---

Originally Posted by Lindley

Deriving from something that isn't meant to be derived from should always be done with extreme care.

---

I agree, but in this case length is virtual.

What really struck me about this scenario is that a derived class can break even though the base class' implementation is completely hidden.

Incedentally, did read that inheritance should not be used fo the sake of code-reuse, but for class-interface-reuse. This seems to add strength to the argument.

Quote:

---

Originally Posted by Zaccheus

Now someone might extend that class to report the length faster. We know the length never changes so we only need to measure it once:

---

The only way you can know that the length never changes when you implement the derived class, is if that is stated in the documentation of the base class. When that changes the semantics of your base class change, which is the reason that the derived class is broken.

If it wasn't stated in the documentation that the length of the string is constant, then you are relying on an implementation detail of the base class in your derived class. When the implementation of the base class changes you're f***ed. That's what you get for trying to write smart code. :p

Quote:

---

Originally Posted by Zaccheus

What really struck me about this scenario is that a derived class can break even though the base class' implementation is completely hidden.

---

What really struck me is that you are strucked :) ... I mean, adhering to documentation (un)specified pre/post conditions it's such a common scenario in C++, isn't it ?

No, I'm not relying on any implementation detail, that's the point I'm making. The implementation can change as much as it likes and the derived class would still work. Given the public interface of the class, the text (and hence the length) never changes, because there is no public way to modify the text.

The problem arises when extra behaviour is added to the base class. So the documentation would need to guarantee that the String class will remain immutable in the future, or should warn that it migh not.

My question was actually more about how the String class can be modified in a way which minimises the risk to derived classes.

I think we all agree that documenting is good, but you cannot always think in advance about what additional features you might add to your class at some point in the future.

Quote:

---

Originally Posted by Zaccheus

No, I'm not relying on any implementation detail, that's the point I'm making

---

this is a rather weak point because it's based on the fact that all member functions are const; in theory, one could add even an unrelated non const method to make the claim "that the string size is constant" an implementation detail ... ( unless explicitly specified by the doc, of course ) ...

anyway, I understand that the issue you're interested in is more general then that ... :)

Quote:

---

Originally Posted by Zaccheus

No, I'm not relying on any implementation detail, that's the point I'm making. The implementation can change as much as it likes and the derived class would still work. Given the public interface of the class, the text (and hence the length) never changes, because there is no public way to modify the text.

---

Well, that depends how the assignment operator is declared in base and derived class. Normally, assigning to a base class does not call the assignment operator of the derived class. In that case the string could change without the length being updated in the derived class. See http://stackoverflow.com/questions/6...ent-operator-c
I get your point, though.

Quote:

---

The problem arises when extra behaviour is added to the base class. So the documentation would need to guarantee that the String class will remain immutable in the future, or should warn that it migh not.

---

I disagree. If the documentation doesn't guarantee that the length of the string will be constant, but you still make that assumption (either by inference or ignorance) you are on your own. It's not practical to document what may change, for the simple reason that you don't know.

Quote:

---

Originally Posted by Zaccheus

My question was actually more about how the String class can be modified in a way which minimises the risk to derived classes.

---

I'd say the only way is through good design. If the functionality of the base class has to be changed, then apparently it was not designed well enough. IMO, the only way to prevent that is through experience. Of course, you can borrow experience to some extend by using established design patterns and coding conventions.

Quote:

---

Originally Posted by Zaccheus

The problem arises when extra behaviour is added to the base class.

---

I would argue that the fact that the string length does not change IS the behaviour. Hence, by allowing your string to change length, you are not adding extra behaviour, but changing existing behaviour. The difference here is fundamental.

As D_Drmmr stated: Either the class is documented as having a specified behaviour (length does not change), and you can rely on it, or the base class does not document the behaviour, and you should not rely on such a behaviour, as it is not guaranteed (in the future, changing string length behaviour may be added).

A typical example would be relying on the fact that vector<T>::iterator is a typedef for T*. It is not documented in the class, and as such, is not guaranteed. However, &myVector[0] is guaranteed to be a T* that points to the first element of your vector. THAT you can rely on.

Bottom line is either the behaviour is documented, and you can rely on it, or it isn't and you can't. Just remember that a behaviour can be the lack of a feature. Anything else is just speculation.

Quote:

---

Originally Posted by Zaccheus

Incedentally, did read that inheritance should not be used for the sake of code-reuse, but for class-interface-reuse.

---

I've read that too, but I don't necessarily go along with it. I have classes where, for example, a generic GUI edit box is the base for a generic numeric edit box, which is the base for int, hex & float edit boxes. Inheriting only from abstract interfaces would be a right P.I.T.A. and involve a lot of replication of code.

The edit box contains the functionality that all edit boxes require.
The numeric edit box adds or modifies behaviour for purely numeric entries.
The int, hex & float edit boxes further specify the behaviour according to the type they support.

I also tend to use inheritance for mixins.

Quote:

---

Originally Posted by superbonzo

... it's based on the fact that all member functions are const ... :)

---

It's based on the fact that there is no way to change the string via the existing public interface. The logical state of the object cannot change, it will always represent the text which was supplied to the constructor.

Quote:

---

Originally Posted by D_Drmmr

Well, that depends how the assignment operator is declared in base and derived class.

---

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.

Quote:

---

Originally Posted by D_Drmmr

I'd say the only way is through good design. If the functionality of the base class has to be changed, then apparently it was not designed well enough.

---

Yes, can't argue against that. Question is, what to do when it does happen.

Quote:

---

Originally Posted by monarch_dodra

Hence, by allowing your string to change length, you are not adding extra behaviour, but changing existing behaviour.

---

Yes that is true.

Quote:

---

Originally Posted by monarch_dodra

A typical example would be relying on the fact that vector<T>::iterator is a typedef for T*.

---

That is an implementation detail. It has nothing to do with the logical state of the object.

Quote:

---

Originally Posted by monarch_dodra

Bottom line is either the behaviour is documented, and you can rely on it, or it isn't and you can't. Just remember that a behaviour can be the lack of a feature. Anything else is just speculation.

---

Quote:

---

Originally Posted by monarch_dodra

Either the class is documented as having a specified behaviour (length does not change), and you can rely on it, or the base class does not document the behaviour, and you should not rely on such a behaviour, as it is not guaranteed (in the future, changing string length behaviour may be added).

---

I could live with that if all classes were adequately documented. As it is, we often have to make certain assumptions based on the public interface of the class.

Quote:

---

Originally Posted by Zaccheus

It's based on the fact that there is no way to change the string via the existing public interface. The logical state of the object cannot change, it will always represent the text which was supplied to the constructor.

---

so, you're saying that "the string size is constant" because "there is no way to change the string via the existing public interface" ? isn't that tautological ? the unique sensible reason ( unless specified by the class doc ) is that all members are const; but as I said that is too weak a condition to be considered an immutable property of that class. I think that most people looking at that class as it is would conclude that "the string size is constant" is an implementation detail.

moreover,

Quote:

---

Originally Posted by Zaccheus

As it is, we often have to make certain assumptions based on the public interface of the class.

---

do you really deduce the logical content of a class from the mere specification of the list of the names of its public members ? it seems an overly optimistic attitude, does't it ?

Quote:

---

Originally Posted by superbonzo

so, you're saying that "the string size is constant" because "there is no way to change the string via the existing public interface" ? isn't that tautological ?

---

Yes, that is exactly what I am saying. It is utterly self evident.

Quote:

---

Originally Posted by superbonzo

the unique sensible reason ( unless specified by the class doc ) is that all members are const;

---

I don't see what the constness of the functions has to do with it. Many languages, like c#, don't even have const functions.

Quote:

---

Originally Posted by superbonzo

I think that most people looking at that class as it is would conclude that "the string size is constant" is an implementation detail.

---

Then we have a very different concept of what implementation detail means. The public interface is *what* the class does/not do. The implementation is *how* it does that.

Quote:

---

Originally Posted by superbonzo

do you really deduce the logical content of a class from the mere specification of the list of the names of its public members ? it seems an overly optimistic attitude, does't it ?

---

It's the first thing I look at, to see what the class offers. I do of course look at whatever documention is available.

Quote:

---

Originally Posted by Zaccheus

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.

Code:

---

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).

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.

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.

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.

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.

Quote:

---

Originally Posted by 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.

---

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.

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.

Quote:

---

Originally Posted by monarch_dodra

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 :D )

Quote:

---

Originally Posted by superbonzo

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 :D )

---

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...

Quote:

---

Originally Posted by 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.

---

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.

Quote:

---

Originally Posted by superbonzo

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? :confused:

Quote:

---

Originally Posted by Zaccheus

Eh?

---

The string object may contain null characters in the middle.

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

Quote:

---

Originally Posted by Zaccheus

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.

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. :(

Quote:

---

Originally Posted by superbonzo

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.