[Discussion] Modifying a class' behaviour - inheritance pitfalls

[Zaccheus]

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

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:

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.

[Lindley]

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.

[TheRogue]

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

[Zaccheus]

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

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.

[D_Drmmr]

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.

[superbonzo]

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 ?

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

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.

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

[superbonzo]

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

[D_Drmmr]

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.

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.

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.

[monarch_dodra]

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.

[JohnW@Wessex]

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.

[Zaccheus]

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

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.

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.

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

Yes that is true.

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.

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.

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.

[superbonzo]

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,

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 ?

[Zaccheus]

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.

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.

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.

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.