On Fri, Feb 25, 2005 at 12:45:34PM -0500, christopher diggins wrote:

----- Original Message ----- From: "Daniel James" <daniel@calamity.org.uk> To: <boost@lists.boost.org> Sent: Friday, February 25, 2005 11:16 AM Subject: [boost] Re: Querying usefulness of a macro for inheriting from STLcontainers

...

christopher diggins wrote: I think publicly inheriting from a class that isn't designed for inheritance is a bad idea. There are type safety problems, non-virtual destructors, slicing etc.

Hi Daniel thanks for the help and suggestions ( and Kevin and Jonathan as well),

I have heard here and there, inheriting from STL container is a "bad idea", but I don't specifically know all the reasons why. I am not an expert in this area (heck or any part of C++ for that matter). Isn't it only bad if someone tries to delete the object using a base class pointer? Could we not

That's one reason, yes. (btw, this is Item 35 in Sutter and Alexandrescu's C++ Coding Standards) It's generally a bad idea to inherit from concrete classes that weren't designed to be inherited from, not just STL containers. In fact, it's generally a bad idea to use public inheritance to model anything except substitutablity. Inheritance creates one of the strngest coupling between two classes you can have, and reducing coupling is usually a good idea. One of my favourite C++ quotes is: [Inlines] are the third most-misused C++ feature (after inheritance and overloading). - Nathan Myers If the class wasn't intended to be a base class it probably doesn't have any protected members, only public and private. If that's the case you don't gain anything by inheriting from it, except the convenience of not havng to declare forwarding functions for all the members - but that's laziness, not good design. By inheriting you _do_ get implicit conversions to the base class, which may not be intentional and may not be safe: void f(const container& c) { container copy(c); // ... } void f(MyCustomContainer& c) { // ... } const MyCustomContainer c; f(c); you probably intended to call the overload taking MyCustomContainer here, but because it requires a const you can't - but instead of an error you get a conversion to base reference, and then the object gets sliced.

prevent that idiom by writing type-casts which force failure upon a pointer cast to the inherited type?

For instance:

public MyCustomContainer : container { operator container*() { throw exception("can not cast to container*"); } ... }

Doesn't this solve the virtual destructor problem?

I don't see how: container* mcc = new MyCustomContainer; delete mcc; No chance for a conversion operator to be invoked here, you never even dereference the pointer, but you invoke undefined behaviour because you don't invoke the right destructor.

Another approach perhaps would be to create inheritable versions of the STL containers which are thin wrappers which simply provide a virtual destructor. Would this not work as well?

If you want to inherit from, say, vector, do you _really_ want _all_ its public member functions?

However what are the other problems of type-safety, slicing(?), etc. to which you allude? I am unfamiliar with them and I am not sure how to go about researching the topic.

Slicing refers to copying the base part of a class only, as in the example above. You pass in a MyCustomContainer, but then copy only a slice of it (the base part). jon -- "You know how dumb the average guy is? Well, mathematically, by definition, half of them are even dumber than that." - J.R. "Bob" Dobbs

Jonathan Wakely wrote:

...

It's generally a bad idea to inherit from concrete classes that weren't designed to be inherited from, not just STL containers.

Jeff Garland wrote:

Well again, to me there are other cases in the design of concrete classes where there is some advantage in inheriting from a base class with a non-virtual destructor. For example in date-time some time duration classes are derive from a base class with no virtual destructor. These subclasses are simply a constructor the adjust for precision. So for example:

[snip]

So a few simple conversion classes help our cause:

class seconds : public time_duration { seconds(int s) : time_duration(0,0,s,0) {} };

class milliseconds : public time_duration { milliseconds(int s) : time_duration(0,0,0,s) {} };

Now we can write:

//code with obvious meaning time_duration td = seconds(10) + milliseconds(10);

You're the designer of the base class - so it is designed to be inherited from. You're not overriding the public interface, so you'll get the same behaviour from the base class and the derived class. I wouldn't feel great about this code, but I find it hard to come up with a really convincing argument against it. I suppose there are alternatives: a function which returns time_duration (but then you can't overload on the type) or a class with conversion operators to time_duration (but that falls down when a function overload requires more user-defined conversions). If you wanted to do something like this, but were worried about it, how much work would it be to remove all possibilities of undefined behaviour? At least under reasonable use, that is. Or is it even possible? I suppose you'd have to make operators &, new and delete protected, anything else? (I'm thinking about something along the lines of boost::noncopyable). Jonathan Wakely wrote:

...

In fact, it's generally a bad idea to use public inheritance to model anything except substitutablity. Inheritance creates one of the strngest coupling between two classes you can have, and reducing coupling is usually a good idea.

Christopher is trying to model substitutability, I think. Jeff Garland wrote:

I'd say there is something about writing less code (not laziness) that makes it a better design. Having to forward or reimplement the whole interface of an stl container is non-trivial and probably a bad design.

In Christopher's use case, he's wrapping most of the member functions anyway. Daniel

Jeff Garland <jeff <at> crystalclearsoftware.com> writes:

On Fri, 25 Feb 2005 18:26:22 +0000, Jonathan Wakely wrote

...

It's generally a bad idea to inherit from concrete classes that weren't designed to be inherited from, not just STL containers.

Well again, to me there are other cases in the design of concrete classes where there is some advantage in inheriting from a base class with a non-virtual destructor. For example in date-time some time duration classes are derive from a base class with no virtual destructor. These subclasses are simply a constructor the adjust for precision. So for example:

class time_duration { time_duration(int hour, int minute, int second, int millisecond) { //calc duration in terms of millisconds } //gobs of other complex stuff like operator+, operator-, etc private: int total_milliseconds; };

Nothing wrong with this, but there's lots of places where writing constructing one of these time_durations is just ugly. For example, suppose I want to add 10 seconds to 10 milliseconds:

//ugly code.... time_duration td = time_duration(0,0,10,0) + time_duration(0,0,0,10);

So a few simple conversion classes help our cause:

class seconds : public time_duration { seconds(int s) : time_duration(0,0,s,0) {} };

class milliseconds : public time_duration { milliseconds(int s) : time_duration(0,0,0,s) {} };

Now we can write:

//code with obvious meaning time_duration td = seconds(10) + milliseconds(10);

Again, we've derived from a class where we don't want virtual functions, we do want to substitite the base class, and it is totally safe because of no allocation / trivial destructor in the sub-class.

Given that you've got these classes called "seconds" and "milliseconds", I'd expect to be able to do things like: seconds s = seconds(10) + milliseconds(20); milliseconds ms = seconds(10); ++s; // increment one second ++ms; // increment one millisecond void foo(seconds); time_duration td(/* ... */); foo(td); Are these things supported? They are all, "code with obvious meaning," (I suppose they could be supported, I don't really know. However, it seems like it would be a lot of work to do, and would only be necessary because of the introduction of the extra classes.) If all you want is to support the creation of simple time units, why not have named functions: time_duration seconds(int secs) { return time_duration(0, 0, secs, 0); } time_duration milliseconds(int msecs) { return time_duration(0, 0, 0, msecs); } We can still write: //code with obvious meaning time_duration td = seconds(10) + milliseconds(10); But no additional classes are needed, and none of the corner cases come up or need to be supported. IMHO, inheritance is often too broad a brush. You get the effect you're after, plus other effects you may not have wanted or anticipated. It's usually a good idea to use as fine a brush as you can get, and that means that if there's some way to implement what you want without using inheritance, you should probably not use inheritance.

...

If the class wasn't intended to be a base class it probably doesn't have any protected members, only public and private. If that's the case you don't gain anything by inheriting from it, except the convenience of not havng to declare forwarding functions for all the members - but that's laziness, not good design.

I'd say there is something about writing less code (not laziness) that makes it a better design. Having to forward or reimplement the whole interface of an stl container is non-trivial and probably a bad design.

I'd say that the achievement of writing (a little) less code at the expense of tighter coupling _is_ laziness. It's actually is pretty trivial to write forwarding functions of an standard container -- so trivial it wouldn't take long to write a code generator for it -- especially when you consider that the whole interface is rarely required. Which brings me to another point about inheriting from standard containers (I'm not sure if this applies to inheriting from other concrete types). In every case I can remember (including cases where I used to derive from containers myself), the desired result was a class that had a more restriced interface and/or behavior than the base (container) class. For example, creating a Polygon class by deriving from std::vector<Point>, then trying to limit the interface that only support the Polygon concept. In every case, achieving this kind of limiting turned out to be easier without using inheritance -- in fact, achieving this with public inheritance turns out to be pretty much impossible, so at some point you have to throw up your hands and decide to live with errors. One lesson I take from this, which I think does apply to (publicly) deriving from concrete classes is: when the semantics of the base class aren't pretty much exactly what you're after, don't derive from a concrete class. Because concrete classes offer no ability to customize or alter the semantics (for example, by providing virtual functions that can be overridden), sooner or later your derived class is going to be used in contexts where it has to act exactly like a base class. For example: void Foo(concrete_class& x); derived_class d; Foo(d); Foo is going to expect its argument to behave _exactly_ like a concrete_class, so instances of derived_class better do so. In my experience, this kind of alignment of semantics between a base concrete class and its derived class is rare. I don't know anything about the date_time library, but I'd be surprised if it were true for time_duration, seconds and milliseconds. [snip]

...

If you want to inherit from, say, vector, do you _really_ want _all_ its public member functions?

Yes, I frequently do.

All I can say to this is that every programmer whom I've ever encountered who answered this question "yes" later changed his mind when we examined the entirety of vector's public interface. I won't go so far as to say "never" publicly derive from concrete classes or standard containers (I always want to leave myself a little wiggle room ;-), but I will say that in all the years I've worked with C++, I can't remember ever seeing a good example of it. Bob

On Sun, 27 Feb 2005 17:52:23 +0000 (UTC), Bob Bell wrote

Jeff Garland <jeff <at> crystalclearsoftware.com> writes: ...snip time_duration example...

...

Again, we've derived from a class where we don't want virtual functions, we do want to substitite the base class, and it is totally safe because of no allocation / trivial destructor in the sub-class.

Given that you've got these classes called "seconds" and "milliseconds", I'd expect to be able to do things like:

seconds s = seconds(10) + milliseconds(20); milliseconds ms = seconds(10); ++s; // increment one second ++ms; // increment one millisecond

void foo(seconds);

time_duration td(/* ... */); foo(td);

Are these things supported?

No, because time_duration doesn't support operator++ or operator--. And actually with the class design it would be pretty easy to extend the subclasses to support this even without offering it in general.

They are all, "code with obvious meaning, " (I suppose they could be supported, I don't really know.

"It's not obvious" is the main reason that operator++/-- aren't supported in date-time. You would have to know the resolution of the time_duration type to understand the code. And since that's a compilation option it seems inherently dangerous to provide operator++/--. Not to mention hard to understand. And really I think: td += milliseconds(10); isn't much more verbose than ++td.

If all you want is to support the creation of simple time units, why not have named functions:

time_duration seconds(int secs) { return time_duration(0, 0, secs, 0); }

time_duration milliseconds(int msecs) { return time_duration(0, 0, 0, msecs); }

We can still write:

//code with obvious meaning time_duration td = seconds(10) + milliseconds(10);

But no additional classes are needed, and none of the corner cases come up or need to be supported.

Sure that probably works, but is there a corner case where it doesn't work? Chained operations maybe? I don't know. In the inheritance design I know it can always converted time_duration. As long as I write interfaces in terms of time_durations all these little adapters can be used in place of the more abstract case. To me this is 'isa' and I don't know of a 'corner case' where this design creates a problem. Another thing. Now that I have an actual type called seconds I can write a function interface or have a data member in terms of that type if I have a reason to be specific about resolution. With a function I can't do that. To me this turns out to be the key for having a type instead of a function. Anyway, I'd be happy to learn if a real actual corner case exists for this design (that's part of the reason I posted). I don't see it, but there are far smarter people than I that read this list...

IMHO, inheritance is often too broad a brush. You get the effect you're after, plus other effects you may not have wanted or anticipated. It's usually a good idea to use as fine a brush as you can get, and that means that if there's some way to implement what you want without using inheritance, you should probably not use inheritance.

Believe me, I agree that inheritence is easy to overuse. But 'broad brush' versus 'fine brush' as a guideline doesn't work. You'll need to define those terms precisely for anyone to use them as guidance. In the example above I don't know how those terms apply. I've read Meyer's guidance and I'm choosing to violate it because I don't think the problems he cite applys and I think the design has advantages.

...

I'd say there is something about writing less code (not laziness) that makes it a better design. Having to forward or reimplement the whole interface of an stl container is non-trivial and probably a bad design.

I'd say that the achievement of writing (a little) less code at the expense of tighter coupling _is_ laziness.

But I WANT to tie my design to the standard library. Why, because it's very stable and most C++ developers know about it. It will allow for seemless client extensions without modification of my class. To me, depending on and offering interfaces using the standard library is an example of a good design coupling.

It's actually is pretty trivial to write forwarding functions of an standard container -- so trivial it wouldn't take long to write a code generator for it -- especially when you consider that the whole interface is rarely required.

But in the real world every line of code written needs to be inspected, tested, etc. 15 years from now someone will need to understand that code. I really hate having to re-write interfaces and such -- it's just not productive.

Which brings me to another point about inheriting from standard containers

(I'm not sure if this applies to inheriting from other concrete types). In every case I can remember (including cases where I used to derive from containers myself), the desired result was a class that had a more restriced interface and/or behavior than the base (container) class. For example, creating a Polygon class by deriving from std::vector<Point>, then trying to limit the interface that only support the Polygon concept.

In every case, achieving this kind of limiting turned out to be easier without using inheritance -- in fact, achieving this with public inheritance turns out to be pretty much impossible, so at some point you have to throw up your hands and decide to live with errors.

This is a bad use of inheritance and I agree inheritence shouldn't be used here. In all my cases, I want a group of custom extensions to the collection related to my domain problem and I want the full power of the collection available to the client.

One lesson I take from this, which I think does apply to (publicly) deriving from concrete classes is: when the semantics of the base class ...snip details In my experience, this kind of alignment of semantics between a base concrete class and its derived class is rare. I don't know anything about the date_time library, but I'd be surprised if it were true for time_duration, seconds and milliseconds.

Well, I came to the conclusion that it is, but I'm open to being persuaded otherwise. BTW in the real date-time library there are other things at play including the fact that the resolution of the time_duration type is a compilation option and hence the adjustment isn't as simple as I showed in the example here. Since the time_duration type defines the resolution the coupling is quite strong.

[snip]

...

...

If you want to inherit from, say, vector, do you _really_ want _all_ its public member functions?

Yes, I frequently do.

All I can say to this is that every programmer whom I've ever encountered who answered this question "yes" later changed his mind when we examined the entirety of vector's public interface.

I won't go so far as to say "never" publicly derive from concrete classes or standard containers (I always want to leave myself a little wiggle room ;-), but I will say that in all the years I've worked with C++, I can't remember ever seeing a good example of it.

class FancyStringCollection : public std::vector<std::string> { public: some_domain_related_utility_function1() {...} some_domain_related_utility_function2() {...} //no added data... }; I know, I should write all these functions domain related utilities as standalone, but you know, most of them just aren't reuseable. They are specific to a very small focus of the design related to the manipulation of this specific collection type. And, I want the interfaces in terms of std::vector<std::string> because I know it's stable and gives the client plenty of options. Often times the extended collection isn't even used as an interface class, but just as a helper to implement an encapsulating domain class. By pulling out the collection related functions I simplify the domain class code and can test the collection related operations independently. Jeff

On Sun, 27 Feb 2005 13:11:11 -0500, christopher diggins wrote

...

At the risk of going against the entrenched dogma here, I'd suggest there are significant cases where this is a good and useful thing to do. I think the original guidance came from Scott Meyers Effective C++ #14 -- make destructors virtual in base classes. The problem I have with this is that I find there are plenty of situations where I might want to add some convenience functions to an stl container in which the subclass has a trivial destructor (eg: no allocated resources)

I am a little confused here, does MySubClass below have a "trivial" destructor?

MySubClass : MyBaseClass { int x; };

In other if I write:

MyBaseClass* p = new MySubClass; delete p;

No leak here. It's only in the case that you allocate in the subclass that there is a problem.

Do I leak the memory for integer x? And then what happens here:

Baz { ~Baz() { cout << "destroying baz"; } };

MySubClass : MyBaseClass { Baz x; };

MyBaseClass* p = new MySubClass; delete p;

Does the baz get destroyed? I always assumed it should.

I didn't try it, but I believe the memory for Baz will be released but the destructor won't be run. It would get bad if you allocated in Baz which would be easy enough to introduce by accident in a real design. Hence the guidance not to go down this path...

...

...

If you want to inherit from, say, vector, do you _really_ want _all_ its public member functions?

Yes, I frequently do.

Jeff

So Jeff, would you find a set of macros for facilitating the inheritance of STL containers useful?

I don't think so. See my other response for how I limit my extensions. Also, I really hate macros for doing these sorts of things because I have to reverse engineer what the macro is doing when I need to read the code. At a minimum that means I need to go to another file, try and grok whether the macro applies to what I'm doing, mentally add in whatever functions/types it creates. I'm getting too old for these sorts of mental gymnastics -- I'd rather have the code just written out. Now if you had policy-based collections that would support full inheritability as an option and implemented standard container interfaces -- that might be interesting ;) Jeff

On Sun, 27 Feb 2005 15:59:19 -0500, christopher diggins wrote

...

Now if you had policy-based collections that would support full inheritability as an option and implemented standard container interfaces -- that might be interesting ;)

Jeff

What kinds of policies would like to see?

Well inheritability is one. Allocation policies that work (I'm no expert but some people think the current standard allocator is flawed). Element ownership would be another. So if I have a collection of pointers and I want them destroyed with the collection. Of course I think we have another boost library that is now covering this space. There is probably some use for a thread locking policy, although it is probably less useful than it might appear since is usually more efficient in classes that wrap standard collections. Once upon a time we had at least some discucssion of this: http://lists.boost.org/MailArchives/boost/msg24761.php Anyway, overall I'm not sure it's really worth going down this path. I'm actually much more interested in seeing things like the tree container in boost. Also, I think you and I probably have unfinished business to make constrained_value a stand-alone library.

PS: if you don't mind debanning me from the wiki my ip address is 66.130.149.217

Done. I got tired of the spam coming from 66.*.*.* so I banned them all -- sorry that was way too aggressive. I've removed all these 'aggressive bans' for now to see how it goes.

PPS : have you considered having a log-in for the Boost wiki?

No, but I wouldn't be opposed to considering such a thing. At some point in the future (maybe middle of this year -- I'm hoping to be less busy then) I plan on upgrading the Wiki software and at that point we might have a discussion on the list about what other Wiki improvements we would like to see. An RSS feed is one thing I've been thinking would be really nice -- although I hear it drives web traffic thru the roof. Jeff

On Sun, Feb 27, 2005 at 01:11:11PM -0500, christopher diggins wrote:

From: "Jeff Garland"

...

At the risk of going against the entrenched dogma here, I'd suggest there are significant cases where this is a good and useful thing to do. I think the original guidance came from Scott Meyers Effective C++ #14 -- make destructors virtual in base classes. The problem I have with this is that I find there are plenty of situations where I might want to add some convenience functions to an stl container in which the subclass has a trivial destructor (eg: no allocated resources)

I am a little confused here, does MySubClass below have a "trivial" destructor?

(N.B. I think the standard uses "trivial destructor" in a stricter sense, meaning that the class *and* all its members and bases have implicitly-declared dtors.)

MySubClass : MyBaseClass { int x; };

I'm not sure what happens to x. I know that with GCC the following code never calls B::~B(), so if it was doing any non-trivial work it would leak: #include <iostream> struct A {}; struct B { ~B() { std::cout << "B::~B\n"; } }; struct C : A { B b; }; int main() { A* p = new C; delete p; } That implies that if you derive from a concrete type with a non-virtual dtor you'd better not add any data members at all. I think don't many people realise that, so I don't think encouraging inheritance from STL containers is a good idea. I might be wrong, but does 12.5/4 mean it is ill-formed to call delete on a pointer-to-base if base::~base is not virtual? jon -- When writing a specialization Be careful about its location Else to make it compile Will be such a trial As to kindle its self-immolation - ISO/IEC 14882/1998 (The C++ Standard)

On Sat, Feb 26, 2005 at 07:55:18PM -0700, Jeff Garland wrote:

On Fri, 25 Feb 2005 18:26:22 +0000, Jonathan Wakely wrote

...

...

...

christopher diggins wrote: I think publicly inheriting from a class that isn't designed for inheritance is a bad idea. There are type safety problems, non-virtual destructors, slicing etc.

Hi Daniel thanks for the help and suggestions ( and Kevin and Jonathan as well),

I have heard here and there, inheriting from STL container is a "bad idea", but I don't specifically know all the reasons why. I am not an expert in this area (heck or any part of C++ for that matter). Isn't it only bad if someone tries to delete the object using a base class pointer?

That's one reason, yes.

(btw, this is Item 35 in Sutter and Alexandrescu's C++ Coding Standards)

At the risk of going against the entrenched dogma here, I'd suggest there are significant cases where this is a good and useful thing to do.

I'm certainly not dogmatic about it and wouldn't turn my nose up at your date-time example, I think it makes sense in that case. Christopher asked about macros to help derive from STL containers specifically, which I do think is usually a bad idea and not something to be encouraged by a Boost library that makes it easy (IMHO). There are some situations, such as Christopher's PwC library, where it might be OK, but I think those situations are rare.

original guidance came from Scott Meyers Effective C++ #14 -- make destructors virtual in base classes. The problem I have with this is that I find there are plenty of situations where I might want to add some convenience functions to an stl container in which the subclass has a trivial destructor (eg: no allocated resources) and as such there is no issue with deriving from container. In all cases I know the base class destructor is called and hence anything the container allocates will be deallocated.

Why do the convenience functions have to be member functions? If you inherit from STL containers you can't be making use of any protected functions or data, since that would rely on details of a particular STL implementation. You might have good use cases for doing it and know what you're doing, but there are plenty of programmers out there who don't. jon

christopher diggins wrote:

Another approach perhaps would be to create inheritable versions of the STL containers which are thin wrappers which simply provide a virtual destructor. Would this not work as well?

You'd also need to think about making all the member functions virtual as well. And consider operator overloading. But it could work. But the STL way to do this is to use the iterators for polymorphism - not the container. That way it won't matter if your class isn't substitutable for std::vector. The OOTL way is up to you. Daniel