extending an STL container class Question

[Graham]

I'm in complete agreement with Yves here. The whole design of the STL is based around extending functionality via non-member functions and decoupling the algorithm from the container. Inheriting subverts that design, increases coupling and decreases the cohesion of your class. There is no need to inherit from a container just to add a function. You add a non-member that takes iterators, and you have a decoupled solution that is extendible and modifiable. It the OP later decides that list<> would have been a better choice of container, the decoupled solution is trivial to change, whilst the inherited solution can be a real pain.

There's also the advice given in Sutter and Alexandrescu's C++ coding guidelines: inherit to be reused, not to reuse. I don't have the book with me at the moment, but I'll check it out tomorrow and expand on that. But for now, they are saying, in essence, that, yes, public inheritance means IS-A, and further, that IS-A implies polymorphism.

[Inspired]

vector<Event> and event_list, however, are classes that do not need to use polymorphism. The interfaces are the same: the OP hasn't redefined a single vector member function. He simply has added functionality in what I believe is the proper way to do so. I mean, why use private inheritance or containment in this case? Then you would need to write code that exposes each public vector member functions that the users of event_list need... what if all of vector's funtionality is needed by the users of event_list?

Anyone who would try to use a vector<Event>* to point to an event_list object would know that the base class is vector. And everyone should know that STL classes do not have virtual destructors. However, as I asserted above, no one would ever need to do such thing, so their knowing or not knowing whether vector has a virtual destructor is not a problem in the first place.

I made it public (originally) because of what you said, needing vector's functionality, which I use elsewhere. I didn`t need a virtual destructor. Or at least I was not aware of it if I did. (I didn`t even know what one was.) However, things did become messy for me, the inexperienced. Going to a function did make things cleaner for me.

[Inspired]

However the philosophy of the STL is that functions acting on the data should be decoupled from the containers. Take iterators and you'll have a much more powerful function than before. This is another point that I think is important to show to people who start out with the STL and which again speaks against public derivation.

I understand now. Thanks

[SuperKoko]

Public inheritance does not always mean polymoprhism. It simply means IS-A.

It is exact.

And if you write some non-polymorphic class and someone else want to use them polymorphically, there are two possibilities:
1) He wants to write some new classes whose base classes are a class you wrote (class B for example). In that case he must derive from class B, and explicitly declare a virtual destructor, and in that case, all classes derived from this new class, can be destroyed polymorphically with a pointer to this base class.
It is normal that someone who wants to use polymorphism must declare this polymorphism.
2) He wants to use polymorphically the already defined hierarchy of classes you wrote:
In that case, even if you defined destructors as polymorphic, he can't, because you have probably not defined the member functions he wants to use, as virtual.

So, if you think that all class hierarchy should be used polymorphically by everybody (and not only the new classes hierarchy somebody could want to write), you must define all functions as virtual, except very few functions whose polymorphism don't have meaning.


About the problem of this thread, i think that the vector should be defined as a class's member of event_list, except if all features of vectors are really needed, but in that case instead of deriving from vector, it is far better to write non-member functions using vector of events.
In fact, public inheritance should be seen as a 'IS-A' relation, but it is even better to see it as 'IS-A, and IS-more than', because if it is not more than the base class, it means that the derived class is written only to add new member functions, and in that case, why the base class cannot use this functions? Okay, in the case of an event vector, you can use the derived class everywhere in your project, and never use directly the base class, in that case the problem is solved, but it is less connectable with some other executables modules that could use the same vectors of events (what, in practice is not really a problem).

[HighCommander4]

HighCommander, you have a point. However, when someone starts out with the STL, I think it's much safer not to recommend public derivation from containers. The risks are there, and if you don't know what you're doing exactly, you'll get into trouble. It's safer not to derive from them, so I think that it's entirely justified to warn against that.

The other thing to take into consideration is that if you want to derive from an STL container, you probably didn't understand the STL's philosophy correctly. Most cases of derivation are there to extend functionality and add new member functions. However the philosophy of the STL is that functions acting on the data should be decoupled from the containers. Take iterators and you'll have a much more powerful function than before. This is another point that I think is important to show to people who start out with the STL and which again speaks against public derivation.

I'm in complete agreement with Yves here. The whole design of the STL is based around extending functionality via non-member functions and decoupling the algorithm from the container. Inheriting subverts that design, increases coupling and decreases the cohesion of your class. There is no need to inherit from a container just to add a function. You add a non-member that takes iterators, and you have a decoupled solution that is extendible and modifiable. It the OP later decides that list<> would have been a better choice of container, the decoupled solution is trivial to change, whilst the inherited solution can be a real pain.

I see your point, but I have 2 concerns:

1) Is it really a good practice to pollute the global namespace with non-member functions, especially ones with a very common name like write(), when you could use a member function instead? That's not to mention the fact that write() is by no means an appropriate name for this function, since it only operates on Event objects. write_event() would be a more appropriate name for it. However, if it's a function that only operates on events, you need some ways of making sure the iterators passed to it yield an Event object when dereferenced. How do you check for that (assuming it's a template function, of course)? And if it's not a template function, then it would need to take vector<Event>::iterator arguments, and then you lose the flexibility of being able to use the function with difference containers, so what's the point? You would get the same coupling as with a member function, but with the added disadvantage of having polluted the global namespace.

2) Even if we accept a non-member function as an appropriate solution in this case, what about other cases where using a non-member function with iterators may not be a suitable solution? What if, say, you want to add functionality that modifies the way the container itself behaves? I stand by my view that in such cases, public inheritance could be appropriate.

[HighCommander4]

There's also the advice given in Sutter and Alexandrescu's C++ coding guidelines: inherit to be reused, not to reuse. I don't have the book with me at the moment, but I'll check it out tomorrow and expand on that. But for now, they are saying, in essence, that, yes, public inheritance means IS-A, and further, that IS-A implies polymorphism.

I would very much like to see these guidelines. If Herb Sutter believes that all public inheritance should mean polymorphism, why does the Standards Committee not make dynamic binding the only option for public inheritance? Or is that one of the revisions they are planning in the new Standard?

I, for one, am a firm believer in the fact that there are situations not suitable for polymorphism where public inheritance is an appropriate way of reusing code.

[wien]

If the ISO Standards Committee would have believed that public inheritance without dynamic binding is risky to use, it would have made dynamic binding the default for public inheritance and left no other alternative. However, the use of dynamic binding has a conisderably high overhead and its overuse can affect a program's performance.

That's not what I'm saying. Public inheritance is useful for many things where virtual destructors (and thus polymorphism) are not needed. The many Boost CRTP classes (like for instance iterator_facade) are excellent examples of that. This is in fact a case where I disagree somewhat with the whole "public inheritance means IS-A" thing. I wouldn't say inheriting from boost::iterator_facade means my class IS-A (Or would it be IS-AN?) iterator_facade, and I certaily wouldn't exploit that polymorphically. The public inheritance in that case is an implemetation detail, and shouldn't even be known by the user.

What I am saying, is that if both the base, and derived classes are in the public and documented interface of the library/whatever you're developing (like in the case of vector and event_list), then the users should be able to use them polymorphically. (Yes, even delete through a base pointer.) If the hierarchy cannot be used polymorphically, you should hide the base class from the users and call it an implementation detail. (Which of course you cannot do with std::vector.)

Sigh.. Maybe it's just me...

One thing I always liked about C++ as a language is that, unlike Java, you don't have to pay for what you don't need.

I quite agree.

This is one of those cases where using dynamic binding is not necessary because no one in their right mind would ever use a pointer to a container, never mind assign a derived class object to it and then try to delete the derived class object through the pointer.

Well... Noone would do that knowing containers don't have virtual destructors. The problem is, not everybody knows that, so why open up for the possibility? Like others have pointed out, there are better ways of extending the functionality of containers. Inheriting from them is just not needed.

The interfaces are the same: the OP hasn't redefined a single vector member function. He simply has added functionality in what I believe is the proper way to do so.

I disagree. The proper way to do so, would be through non-member functions.

Anyone who would try to use a vector<Event>* to point to an event_list object would know that the base class is vector. And everyone should know that STL classes do not have virtual destructors.

In this specific case most people would know that, yes. But that doesn't make it okay. I don't think this should be required information for people simply using your vector inherited class.

In the more general case, if I made a class with no virtual destructor (thereby signalling "don't inherit from this one"), and you did anyway, other programmers would not know that, and they would probably assume polymorphism is safe. (I certainly would) Why should people have to check my base class for a virtual destructor before they use it polymorphically?

However, as I asserted above, no one would ever need to do such thing, so their knowing or not knowing whether vector has a virtual destructor is not a problem in the first place.

How do you know that? Someone might need to do that... You just don't know.

I think the central idea here is that there is a difference between public inheritance and polymoprhism. Public inheritance does not always mean polymoprhism. It simply means IS-A.

Yes.. And in my opinion IS-A means polymorphism, but public inheritance does not *always* mean IS-A (Sorry Graham! ) That's where we disagree I guess.

The reason the STL writers did not make the STL containers have virtual destructors is because they saw the containers unfit for use is a polymorphic situation (not because they saw them unfit for public inheritance - there is a difference).

Are you sure that is really what they meant? I would really like to see a quote to that effect if they do. As far as I have been able to gather, no virtual destructor means "do not inherit".

Yet you try to question this, and manage to come up with these highly unlikely scenarios where people would go against the design intended by the STL writers and try to (wrongly) use STL containers in a polymorphic way. Well, bad design has its consequences. In this case, trying to use vector, a well-known class in the standard library, polymorphically even though it's clearly not meant to be used so, may lead to undefined behaviour. So why worry about the trouble people get into when they use a bad design?

The point is that you are shifting a big resposibillity over to the users of your class. If everyone did that we would have to check any base classes for virtual destructors every single time we use them polymorphically. Do you do that?

If you instead just say IS-A means polymorphism and that public inheritance only means IS-A if the base class is available to the users, then they just don't have to do this anymore. It takes away quite a few possibilities for errors made by programmers. (Always a good thing)

Phew.. I don't know. Am I making myself a little clearer now?

[wien]

Is it really a good practice to pollute the global namespace with non-member functions, especially ones with a very common name like write(), when you could use a member function instead?

You could always make a namespace and put the fuction in it. Or just name it better.. write_event, like you said yourself.

However, if it's a function that only operates on events, you need some ways of making sure the iterators passed to it yield an Event object when dereferenced. How do you check for that (assuming it's a template function, of course)?

Boost has an excellent solution for that. Static assertions. You could just assert that std::iterator_traits<T>::value_type equals Event, and trying to pass anything other than that would fail at compile time. This would work no matter what iterator type T is...

[HighCommander4]

Are you sure that is really what they meant? I would really like to see a quote to that effect if they do. As far as I have been able to gather, no virtual destructor means "do not inherit".

It's a simple logical conclusion. Publically inheriting from a class with no virtual destructor does not result in undefined behaviour. Using a class hierarchy where the base class does not have a virtual destructor, polymorphically, may result in undefined behaviour. Hence, having no virtual destructor is a signal that it is unsafe to use a hierarchy with this class a base class, polymorphically, *not* that it's unsafe to publically derive from this class.

What also follows from this conclusion is that if someone tries to use a class hierarchy where the base class does not have a virtual destructor, polymorphically, the consequences of this are entirely their responsibility, because they are the ones who are not using the hierarchy the way it is intended to be used, not whoever wrote the derived class(es).

The point is that you are shifting a big resposibillity over to the users of your class. If everyone did that we would have to check any base classes for virtual destructors every single time we use them polymorphically. Do you do that?

If you instead just say IS-A means polymorphism and that public inheritance only means IS-A if the base class is available to the users, then they just don't have to do this anymore. It takes away quite a few possibilities for errors made by programmers. (Always a good thing)

wien, I respect your viewpoint on this issue but I still think that not every publicly available class hierarchy needs to be used polymorphically. Some do, some don't. In either case, the intentions of the code writers should be well documented, so that the user does not have to infer these intentions by looking at destructors etc. If a user is unable to tell whether or not a class hierarchy should be used polymorphically, it's a sign of poor documentation, not of poor design. I completely agree with you that no one should have to take a look at whether or not a base class has virtual destructors to be able to determine whether or not to use the hierarchy polymorphically. But having to enable dynamic binding, thus incurring considerable run-time oberhead, for every single publically available class hierarchy even if it is one that does need polymorphism (for example, vector<Event> and event_list), just because there are some people who do not bother to read the documentation and who blindly use others' code for whatever need they see fit without consideration of the code writers' intentions, is quite unreasonable.

[Graham]

I present this purely for information:

C++ Coding Standards, Sutter and Alexandrescu:

35. Avoid inheriting from classes that were not designed to be base classes.
[...]Using a standalone class as a base is a serious design error and should be avoided. To add behavior, prefer to add nonmember functions instead of member functions (see item 44). To add state, prefer composition instead of inheritance (see item 34). Avoid inheriting from concrete base classes.
[...]

37. Public inheritance is substitutability. Inherit, not to reuse, but to be reused.
[...]Don't inherit publically to reuse code (that exists in the base class); inherit publically in order to be reused (by existing code that already uses base objects polymorphically).
[...]

44. Prefer writing nonmember nonfriend functions.
[...]Where possible, prefer making functions nonmember nonfriends.
[...]Nonmember nonfriends improve encapsulation by minimizing dependencies: the body of the function cannot come to depend on the nonpublic members of the class.[...]

[marten_range]

Boost has an excellent solution for that. Static assertions. You could just assert that std::iterator_traits<T>::value_type equals Event, and trying to pass anything other than that would fail at compile time. This would work no matter what iterator type T is...

For those interested:

boost also has something called concepts that is used for example by boost::graph.

Some algorithms in boost::graph works for example only on bi-directional graphs. boost::graph is heavily templatized and in order to both make sure that the graph that is passed to the algorithm is a bi-directional graph and to provide user-friendly error messages (compile-time) if it's not boost::graph uses concepts.

Concepts basically are more high-level static assertions.

http://www.boost.org/libs/concept_ch...cept_check.htm

[NMTop40]

Actually your write_event looks like it would best suited as a function-object. Then you could call

std::for_each( v.begin(), v.end(), MyEventWriter( my_outstream ) );

Alternatively if you want to create MyEventWriter so you still have a copy of it after the std::for_each you can create it first, thus:

MyEventWriter evwr( myOutStream );
std::for_each( v.begin(), v.end(), evwr );

If you don't want to use operator() in evwr but a named member function instead, you can use bind.

[exterminator]

What also follows from this conclusion is that if someone tries to use a class hierarchy where the base class does not have a virtual destructor, polymorphically, the consequences of this are entirely their responsibility, because they are the ones who are not using the hierarchy the way it is intended to be used, not whoever wrote the derived class(es).

I would not agree to this. If I am constructing something I should try as much as possible that people dont mess up with my product. Taking a simple example - getters and setters - if I dont wish to provide 'set' access to someone I won't provide a setter method and make my data member private. Its not that I would make it public and in the document I would write that this member should not be set !!! This does not make any sense.

By the way, What I would say here is that if a particular class is not meant for inheritance (or does not have virtual destructors) we should have a mechanism to not allow their derivation in the first place. And if we are allowing public inheritance there should be no loop-holes about usage, be it polymorphic .. be it non-polymorphic. But again I would agree to HighCommander's quote - "In C++ you don't have to pay for what you don't need." Now, this again asks for allowing base classes publicly inheritable without virtual destructors.

...just because there are some people who do not bother to read the documentation and who blindly use others' code for whatever need they see fit without consideration of the code writers' intentions, is quite unreasonable.

In my opinion, Agreed!!! Completely. How could you use others' code without reading the documentation. But I would also add that its not always practical. You might be lacking documentation. You might even get enhancement work in which you dont have them in hand. There can be various such cases in the real world scenario. In some places, where processess are not implemented people dont even do any documentation!!!

I guess there's no particular conculsion to this. This basically depends on the possibilities of usages of ur code. The degree to which they would be re-used. The life-time of the software, in a general sense.

[Bob Davis]

But having to enable dynamic binding, thus incurring considerable run-time oberhead, for every single publically available class hierarchy even if it is one that does need polymorphism (for example, vector<Event> and event_list), just because there are some people who do not bother to read the documentation and who blindly use others' code for whatever need they see fit without consideration of the code writers' intentions, is quite unreasonable.

This obviously has become quite a passionate discussion. I think you need to take a step back and think for a moment: what do you gain by publically inheriting from vector in the first place? OK, so it seems more "natural" to inherit from the container, since an event_list IS-A vector. But look at what you lose:

1. Genericity of underlying container type.
2. Clarity of your intentions and the underlying implementation.

These things, and especially #2, cannot be ignored when you're working at a "real" job where intuitive code re-use means less programming time, which means shorter schedule and less cost, which is good for everyone. I wholeheartedly agree with wien in this case, as you can't be sure that someone will read your documentation, or if they will even have access to it. It doesn't make sense to buck the trend of conventional C++ design principles to make something that seems more natural to you, especially when there is an alternative that will prevent confusion and also grant you the ability to change containers in the future. Write code to be reused, not just by you, but by any of your colleagues; you'll be glad you did.

[NMTop40]

You can write a class that cannot be inherited from by giving it private constructors and a friend or static member to construct one (which would also force heap construction, because the copy-constructor would also have to be private).

In Java they have final but then Java also enforces heap-construction.

Of course, you could get your constructor to return you a reference-counted pointer rather than a raw pointer so you won't have to worry about calling delete. And it wouldn't have to necessarily be a standard reference-counted pointer, but could be a custom-made one.

But now you'll have problems, thus:

- Does it use COW or share the data when copied?
- Who is to stop someone deriving from your reference-counted pointer?