| Date: | September 13, 2006 / year-entry #310 |
| Tags: | code |
| Orig Link: | https://blogs.msdn.microsoft.com/oldnewthing/20060913-04/?p=29763 |
| Comments: | 30 |
| Summary: | When should you mark a method as virtual? This question has both a technical and a philosophical aspect. The technical aspect is well-understood: You mark a method as virtual if you want calls to the method to be invoked on the run-time type of the invoked object rather than on the compile-time type of the... |
When should you mark a method as virtual? This question has both a technical and a philosophical aspect. The technical aspect is well-understood: You mark a method as virtual if you want calls to the method to be invoked on the run-time type of the invoked object rather than on the compile-time type of the invoking reference. But there is a heavy philosophical aspect to this question as well. Some people argue that every method should be virtual so that derived classes can override the behavior of the base class in ways the base class may not have anticipated. This grants you maximum flexibility, avoiding having to redesign the class in the future. I happen to believe that the cost of this extensibility is badly underestimated. Once you mark a method as virtual, your life becomes much more complicated. Every virtual method is an extensibility point, which means that you have to assume the worst. You can't hold any locks when calling a virtual method (else you may deadlock with the derived class). You have to be prepared for all sorts of reentrancy because the derived class may have decided to call It takes only a few seconds to type the word "virtual" but you need to know what you just signed yourself up for. I'm not saying that virtual methods are bad; I'm saying that you need to understand what you are doing. You don't say "Gosh, somebody might want to override this, so I'll make it virtual." You have to say, "Gosh, I'm creating an extensibility point here that needs to be designed and tested." Of course, if your class is private, then you are the only person who can derive from the class, and therefore you can impose rules on what the derived classes are permitted to do if they override a method, and you have the power to enforce said rules on those derived classes since you're the one who wrote them anyway. But if you allow others to derive from your class, then you have entered the world of extensibility and your life has just gotten a lot more complicated. |
Comments (30)
Comments are closed. |
I tend to do the extreme opposite, and write classes to explicitly disable derived classes–and even operations like assignment–unless I have thought through the implications. That allows me to be lazy and only do the design changes when I actually need the functionality and flexibility. At that point I usually have a better idea of what is really needed anyway.
That sort of approach won’t work for writing a public class library of course. My condolences to those guys.
My rule of thumb is to avoid making public methods virtual. Because if I have a public virtual method and some time later I notice "oh, there’s some code that should be called shortly before or after any code in the derived class is called", I’m out of luck.
That’s why I tend to use a pattern something like this:
protected virtual void OnPerformAction()
{
// default implementation
}
public void PerformAction()
{
// code may be added later here…
OnPerformAction();
// … or here
}
Seems like a metaphor for Win16 -> Win32 -> Win64…
This discussion has many different points of view and ways of being argued – it all depends on the purpose of your class.
For some objects, it does not make sense to derive from them, so virtual functions are pointless.
For other objects, the name of the game is polymorphism and virtual functions are essential.
I take pretty much the opposite view of Roland and make most of my public functions virtual (if it makes sense to do so).
One thing that bugs me is function hiding when you forget to declare a function as virtual.
I think the C++ compiler has a long way to go to keep us programmers from tripping over our code.
I guess I’m more concerned with the security aspects. A ‘contract’ or ‘intent of the method’ is out the window if the deriver’s intent is malicious.
I know this isn’t a .NET blog but that is my world at this point. With dynamic loading and factory patterns and such, I can see where someone might be able to get malicious code injected into an application.
Not a point of contention here – just one of the things I take into account when making the virtual/non-virtual decision.
>> the derived class may have done "crazy" things inside that virtual method
Usually there is some kind of "contract" (which should be documented but has some implicit points) which derived class must respect.
If the method I call is called ReadData and the base class just reads data, the derived object clearly should not delete itself inside that method (unless explicitely allowed).
An autodeletion inside a method which clearly should not autodelete is as evil as starting a thread which reads from char*p=(char*)(0) at a random time.
I think the Liskov Subsitution Principle is a good criteria to decide if the functions of a class are to be marked virtual or not. If a derived class can’t be seamlessly subsituted for a parent class or a sibling class, then the parent class is probably not a good candidate as parent class with virtual functions.
In other words if the client object or function needs to know something about the internal mechanism of the object of a derived class through a base class pointer, then the LSP is being violated, and using polymorphism is probably wrong.
In my own (very limited) experience the LSP is pretty hard criteria to satisfy, but has been a nice guide to simplify my designs.
> "I guess I’m more concerned with the security aspects. A ‘contract’ or ‘intent of the method’ is out the window if the deriver’s intent is malicious."
I certainly understand that approach for object reuse in the COM sense, because the environment can be hostile and the attacker misusing your component isn’t the one who owns the computer under attack.
But this is source code. Why would someone be trying to derive a class with *malicious* intent? It’s easier to be directly evil in your own code, or by copying and modifying the original source code to change its behavior.
Admittedly, someone might try to use your unmodified source as an obfuscating evil proxy for their own bad actions, but incompetence and carelessness has got to rate a lot higher as a cause of problems.
I’m not sure Brian’s argument about malicious code makes much sense. If someone manages to inject malicious code into your app using any of the aforementioned methods, the changes are that any state validation you do won’t help much. They can potentially patch the code doing the validation (except in the presence of DEP / W^X or equivalent), never return control to your app at all, or do almost anything else they want. In fact, we saw a good discussion of this issue just recently here in the New Old Thing with the post on using the return pointer as a caller security mechanism in DLLs.
If you’re talking about an IPC interface like COM, then a certain level of paranoia and lack of reliance on documented "contracts" does make good sense. For in-process calls I don’t think it does.
Personally I tend to include such state checks before and after virtual method calls, but they’re `const’-qualified and compiled in only for debug builds. This helps the subclass developer retain their sanity if they miss a documented requirement of the interface or my documentation is lacking, because the sanity checks will often detect their error. However, it imposes no large costs on the production builds. Cheap checks are often still included to make tracking down problems in deployed code easier.
When defining a method as virtual, there is an understanding on why this must be virtual. The virtual method may be expected modify the state of the object. By defining a post-condition (http://en.wikipedia.org/wiki/Postcondition) for a virtual method, we may have some control on what the method does.
Other than Eiffel, is there programming langauges that support pre and post-conditions for virtual methods in some form or the other?
A good option would be to have attributes for pre and post conditions.
class C1
{
private int a;
[Pre-condition: a > 0 && a < 100;]
[Post-condition: a > 0 && a < 1000;]
protected virtual void ActOnA();
};
I’m with Norman Diamond here. I make almost all methods virtual, because at some later stage having a virtual method may be convenient. (I like Roland’s pattern, and may adopt that in some cases. Thanks.)
I think the primary thing is to document what state the application may be in when the method is called. If locks may be held, document that (assuming you don’t have some kind of global deadlock avoidance strategy). My general feeling is that any methods designed to be called from the message loop should be reentrant anyway: you might want to stick a MessageBox in there for debugging purposes anyway, and coping with reentrancy isn’t normally difficult. But if it isn’t reentrant, just document that the method cannot do anything that might result in a message being dispatched. Sure, it limits the usefulness of being able to override the message, but it’s still better than having to go back and make a public interface change if you find you need to at a later stage.
I think the key thing here is, make sure everyone knows what’s going on. If you do that, it should work out OK.
Just my experiences, which may be somewhat influenced by the fact that I work at a small shop (only 2 programmers).
Interesting to compare java where everything (non static) is virtual unless you make it final, with c# where eveything isn’t virtual unless you explicitly make it so.
I like the c# approach – I prefer deciding whether to open up a class as opposed to remembering to close it off.
I’m 100% with Raymond on this one, and believe that those who believe that every function or class should be virtual ("to support reuse", what a lame excuse) are not making effective programs (neither easy to maintain nor fast to execute). When they make programs for themself, I guess they see nothing wrong. Still I wouldn’t like them in my team.
All these rules like "every function should be virutal" or "all variables should be private" are plainly bad. A lot of what was preached in multitude of books since C++ got popular came from people who never developed anything bigger than "hello world".
When I’m by C++ problems, I consider the whole iostream C++ standard library a major flop, that demonstrats a lot of problems inherent in C++. Stroustrup should have developed C++ a little more to support something better.
@Jules:
The "Roland’s pattern" is commonly known as "template method". See http://en.wikipedia.org/wiki/Template_method_pattern (or pick up the GOF book :)
"Every function should be virtual" is an appealing opinion, but it is also very idealistic – which doesn’t mix well with the highly pragmatic nature of C++ itself.
Java, Smalltalk, Ruby and other languages which to various extents aspire to be "fully" object oriented are a different beast altogether – functions being "virtual" by default (or that being the only option) makes sense in those languages.
When programming in C++ I use virtual functions pretty much exclusively to define interface-type (in the java/C# sense) classes, most of which are abstract. IMHO that’s the single most useful application of inheritance – otherwise I do just as well without. Even the most trivial of textbook examples emphasize the value of being able to treat apples and oranges as fruit.
Slightly off-topic, I just have to make a point: C++ isn’t a very dynamic language. Violent attempts to make it behave and seem more dynamic than it really wants to will only make you frustrated and unhappy. Mmkay?
>>> I totally agree. Its gets me mad when I see devs declaring the d’tor of their newly created class virtual before any. On a code review when you ask them why they did this they usually say: "Because all d’tors must be virtual". <<<
I *always* make destructors virtual (except for classes that are used in a completely private context). There is a very good reason for doing this: if a subclass of the class is added later, and an instance is used as a substitute of a parent class instance, it is possible for the wrong destructor to be called. The slightest chance of that happening is truly bad.
I’d like to point out that this problem, in C++, is both simpler and more complicated than it seems at first. Consider these (legal C++) declarations:
class A { … };
class B : public virtual A { … };
Regardless of what your intentions were with class A, regardless of what semantics you might have wanted it to have, every one of its methods are treated as virtual by class B. So the choice of "virtual or not" in C++ is a bit misleading: Anybody who really wants to can subvert your decision, no matter how wise your decision may have been.
Because of this, I tend to lead more toward non-virtual than toward virtual (in fact, in light of the ability to do this, declaraing every method of a given class as "virtual" is silly and meaningless). Since my choice can be overridden anyway by anyone so inclined, I treat the "virtual" keyword as a hint to other programmers as to the intent of my design: Methods without "virtual" were probably not intended to be overridden, while any method marked "virtual" has the full blessing of the original designer to be overridden.
Sean, virtual base classes are different than virtual functions. Virtual base classes are used for multiple inheritance when those base classes are derived from a common base. This makes the derived class only contain a single instance of that common base class.
There is no way that deriving a class from A can make a function call on a pointer to A from a plain function to a virtual function. The compiler generates completely different code for the two different types of calls.
Thursday, September 14, 2006 6:36 PM by Sean W.
> perhaps "class A : common public B" would
> have been a better term,
The C/C++ philosophy is to reuse keywords as much as possible instead of defining new ones, deriving new uses for base keywords, but implementing new meanings that don’t derive from the old meanings.
But you’re right that this reuse of "virtual" was a bit too counterintuitive in its meaning.
So they should have called it "class A : static public B" ^_^
Hmm, you’re right, Greg. My mistake.
(Or possibly a lousy word choice on the part of the C++ designers; perhaps "class A : common public B" would have been a better term, given what the "virtual" keyword does in that circumstance. But, since I don’t use multiple inheritance, this explains why I’ve never found a need to stick a "virtual" in there before.)
Still, I think the rationale that results from that misconception was pretty good, and I still follow that model in my own code. If you don’t mark a method as virtual, anybody who really *wants* to get around that limitation can usually find ways to do so (even if not exactly the same as if it had been marked virtual). I really think virtual should be reserved for methods where you’re giving your approval to other people to override them, not simply "just in case" (which can and does cause problems later in the design). Everything in moderation, "virtual" included.
>I *always* make destructors virtual (except for classes that are used in a completely private context).
>There is a very good reason for doing this: if a subclass of the class is added later, and an instance is used as a substitute of a parent class instance, it is possible for the wrong destructor to be called. The slightest chance of that happening is truly bad
Many classes don’tr even need an d’tor. If a class just holds two doubles it doesn’t need a d’tor
Of course in most cases it is totally right to declare a dtor virtual and therefor the claswizard has a good default by making all d’tor virtual. But not for every class. Hey it’s C++ it’s about having the choice! Its just that I would like people _think_ before they declare something virtual.
Thats what we’re paid for :-)
Thinking is laborious.
hi,
I’m a Spanish student and I have read your comments and I’d like to comment anyone have thought in this sentence:
>>> "… that needs to be designed and tested." <<<
We must remember that anything we do, it must tested. We, as software engineers, mustn´t only in the design; and we must think in the consequences, such as testing. In this stage, we have to test the security in our applications and other type of tests; and anything type of test means spending much money.
Of course, we mustn’t forget that everything has more point of view and nothing has a only solution.
Att,
Alberto.
—
Albertito (aka keiko)
Página Personal: http://atetinho.googlepages.com
Blog: http://ifelsedeveloper.blogia.com
C.A.S: http://www.ajedrezsanturtzi.com