Hi Markus,
I will be honest, I have never understood why the destructor should be protected and non-virtual. Even I had read the articel again. [snip] But maybe you can it explain more in detail.
The issue this warning addresses is that according to the standard the behavior of the program is undefined if you delete an object using a pointer to a base class that has not declared a virtual dtor. But whether or not we need a virtual dtor depends on our situation - we could have a hierarchy of polymorphic types, or we could have a non-polymorphic (static) hierarchy. No need to pay the price of polymorphism if we never intend to use it!
Declaring the dtor of the base class protected would mean that only derived classes would be allowed to call it (indirectly from their own dtor). Therefore the user of a class will not be able to delete an object pointed to by a Base *, so having a non-virtual dtor is not calling for any trouble - you would only be allowed to delete an object with a pointer to the derived class so you’ll get the correct ctor called (unless you did some weird casts) without need of polymorphism.
That you should never derived from a class with a non-virtual destructor comes from Scott Meyers new book “Effective C++”
I love this quote: “Whenever someone says to you, “You should always make data private,” stop right there — it’s an “always” or “never” rule, and those rules are what I call one-size-fits-all rules. The real world isn’t that simple.” from http://www.parashift.com/c++-faq-lite/basics-of-inheritance.html. I think it’s a real big problem that all these rules exist - it would have been much better if they were called and considered guidelines. But we all also know that for each rule there is an exception, some rules even have more than one ;-).
Back to my BinaryReader example, I think it would actually be better design if I rename Utils::Encodings::BinaryReader to BinaryReaderBase and add it a protected dtor. Then I would simply derive a new Utils::Encodings::BinaryReader class that derives from BinaryReaderBase. I’ll also have the BinaryReader classes from Gnutella and BitTorrent derive from that base class. That design will still produce the warning, but after we’ve learned from it and have improved our design, we could safely ignore it. :-)
Regards,
Peter
