C++ Style: Prefixing virtual keyword to overridden methods
Asked Answered
A

6

27

I've been having a discussion with my coworkers as to whether to prefix overridden methods with the virtual keyword, or only at the originating base class.

I tend to prefix all virtual methods (that is, methods involving a vtable lookup) with the virtual keyword. My rationale is threefold:

  1. Given that C++ lacks an override keyword, the presence of the virtual keyword at least notifies you that the method involves a lookup and could theoretically be overridden by further specializations, or could be called through a pointer to a higher base class.

  2. Consistently using this style means that, when you see a method (at least within our code) without the virtual keyword, you can initially assume that it is neither derived from a base nor specialized in subclass.

  3. If, through some error, the virtual were removed from IFoo, all children will still function (CFooSpecialization::DoBar would still override CFooBase::DoBar, rather than simply hiding it).

The argument against the practice, as I understood it, was, "But that method isn't virtual" (which I believe is invalid, and borne from a misunderstanding of virtuality), and "When I see the virtual keyword, I expect that means someone is deriving from it, and go searching for them."

The hypothetical classes may be spread across several files, and there are several specializations.

class IFoo {
public:
    virtual void DoBar() = 0;
    void DoBaz();
};

class CFooBase : public IFoo {
public:
    virtual void DoBar(); // Default implementation
    void DoZap();
};


class CFooSpecialization : public CFooBase {
public:
    virtual void DoBar(); // Specialized implementation
};

Stylistically, would you remove the virtual keyword from the two derived classes? If so, why? What are Stack Overflow's thoughts here?

Aschim answered 3/9, 2009 at 1:10 Comment(5)
I'd be interested to see Bjarne's (or the C++ committee's) rationale for why virtual is even allowed to be omitted in the derived class. The rationale might provide more compelling reasons for actually omitting it than does your colleague. Perhaps only in certain cases, though.Fisher
Unfortunately there is no rationale included in the spec. It just says it is "legal but redundant (has empty semantics)". Would be interesting to hear, though, if there is any record of what the rationale was.Post
I don't have a copy of "design and evolution", which may or may not contain anything about it.Fisher
I'd like to point out that as of C++11, you can also use the override specifier to, well, specify that a function is virtual and overrides another function; doing so also allows the compiler to give you an error message if the function doesn't override anything. With your example, CFooBase and CFooSpecialization would have virtual void DoBar() override;.Certification
@JustinTime I think what people want, as impractical as it is, is a Standard-enforced way to require descriptive vfunc declarations, rather than having to remember to deliberately type yet another keyword that is totally optional without having to turn on compiler-dependent warnings. But I think it's too late for that now in terms of what it'd break and the customary backlash that would result. That said, here's the trick for g++: #29145976Hellfire
P
23

I completely agree with your rationale. It's a good reminder that the method will have dynamic dispatch semantics when called. The "that method isn't virtual" argument that you co-worker is using is completely bogus. He's mixed up the concepts of virtual and pure-virtual.

Post answered 3/9, 2009 at 1:12 Comment(0)
K
6

A function once a virtual always a virtual.

So in any event if the virtual keyword is not used in the subsequent classes, it does not prevent the function/method from being 'virtual' i.e. be overridden. So one of the projects that I worked-in, had the following guideline which I somewhat liked :

  • If the function/method is supposed to be overridden always use the 'virtual' keyword. This is especially true when used in interface / base classes.
  • If the derived class is supposed to be sub-classed further explicity state the 'virtual' keyword for every function/method that can be overridden. C++11 use the 'override' keyword
  • If the function/method in the derived class is not supposed to be sub-classed again, then the keyword 'virtual' is to be commented indicating that the function/method was overridden but there are no further classes that override it again. This ofcourse does not prevent someone from overriding in the derived class unless the class is made final (non-derivable), but it indicates that the method is not supposed to be overridden. Ex: /*virtual*/ void guiFocusEvent(); C++11, use the 'final' keyword along with the 'override' Ex: void guiFocusEvent() override final;
Kerin answered 3/9, 2009 at 3:52 Comment(0)
R
4

I would tend not to use any syntax that the compiler will allow me to omit. Having said that, part of the design of C# (in an attempt to improve over C++) was to require overrides of virtual methods to be labeled as "override", and that seems to be a reasonable idea. My concern is that, since it's completely optional, it's only a matter of time before someone omits it, and by then you'll have gotten into the habit of expecting overrides to be have "virtual" specified. Maybe it's best to just live within the limitations of the language, then.

Regenerative answered 3/9, 2009 at 1:38 Comment(23)
"I would tend not to use any syntax that the compiler will allow me to omit." - whereas I think there are some strong cases for using keywords which I'm permitted to omit, for clarity. For example, if I group class members together, then I put an access specifier at the start of each group even if it's the same as the specifier for the previous group. I put "virtual" in the same category - it's so common in my experience to miss that something is virtual, that the value of adding it outweighs the cost. It's really just a comment, provided the base really is (and remains) virtual.Fisher
Does that mean you don't use "inline", "const", "volatile" and "explicit"? Note that I'm not going to pull out the old dogs "auto" and "register" (don't recall if they even made the transition from C to C++, perhaps not, but that's fine, I've never used them in 20+ years anyway). If your intent is "I don't use any syntax that has no effect on the code" that's a little different. Not trying to be pedantic.Cappadocia
I keep hearing from various people, that C# is an attempt to improve over C++. It's a totally different question, I guess, but how can a language be an improvement if it's significantly more complicated, has a lot more syntax constructs (LINQ anyone?) and is so tightly tied to it's standard library (it took me quite some time, coming from C++, to understand that int[] is a class that inherits from IEnumerable<int> etc.)?Jewish
"auto" and "register" are part of C++ (though with little use) - "auto" will gain new life in the next version of C++ as the keyword for declarations to use for type inference..Alyce
@Jewish - this is the first time I've heard someone complain that C# is more complicated than C++.Alyce
@onebyone: I think clarity is best served by not repeating yourself or explicitly specifying the default. So, for example, I see little purpose to stating that a method is private when, by default, all methods are. On the other hand, there are places where it's syntactically necessary to specify it, and there I do. Essentially, I am trying to cut down on the excess verbiage (in my code, clearly not in comments here!) so that we can focus on what it does, not what the compiler wants me to say.Regenerative
@Dan: Yes, thanks for the clarification. When I say I would avoid syntax that the compiler would allow me to omit, I do indeed mean "omit without any change in behavior". Clearly, things like volatile and const do affect behavior, so they don't count. But virtual on an override is just seasoning, and I prefer not to over-season.Regenerative
@Paulius: C# has its own new complexities, but I was referring to changes in the basic OOP declarations, such as requiring that you specify either "new" or "override" when your signature matches a method in a parent class. C# got a few things right that C++ didn't, including making "this" a reference rather than a pointer. Please don't take this as the opening salvo of a pointless C++ vs. C# debate: I've programmed extensively in both and they each have their niche.Regenerative
Fair enough, but with defaults if you don't specify, then the reader has to work it out from scratch (in the case of virtual, by looking at all base classes). If you do specify, then the reader knows without any further work, but of course if the default changes you don't get the new default. So I think on a case-by-case basis you should decide which is more valuable, rather than having an assumption that specifying defaults is more wordy and hence inherently less clear. C++0x is going to introduce ~MyClass() = default; precisely in order to let people like me have their way :-)Fisher
Of course, we can always comment the derived class declaration to say that it's virtual, or say that it's non-virtual. Readers can then decide whether they trust us enough to believe the comment without checking...Fisher
@onebyone: I guess my concern here would be that, since it's entirely optional, the compiler cannot enforce its usage, so it's only a matter of time before we're inconsistent. Imagine the case where Foo has a dozen methods, two of which are virtual, and its child class dutifully labels just one as virtual. A reasonable programmer might think the omission of the keyword could be relied upon as an indication that a method isn't virtual. On the other hand, if the keyword were used only where required, they would know to check the base instead of trusting an optional notificationRegenerative
Yes, I see what you mean. I think I count on the reader to be paranoid - if it's marked virtual then it's virtual. If it's not marked then you're on your own (i.e. shouldn't assume non-virtual). But that's probably too optimistic, even if the reader is myself later on. And even if it's marked virtual in the derived class, it doesn't mean it's virtual in the base class, which means calls via a base class reference might still be non-virtual. I think this is just irreducibly difficult in C++, so it's a game of percentage chances of different errors.Fisher
@onebyone: Yes, that's why I think C# got it right by requiring "override". You also bring up an excellent point that I had overlooked: it's entirely possible that the "virtual" marks a method that did not exist in the parent class, so slapping that modifier on overrides serves to hide this distinction. I guess this is a fine example of how C++ is powerful but a big ugly.Regenerative
And you could maybe argue that in a given class, either all (non-private, non-static) functions should be virtual, or all should be non-virtual, or they should be separated using the NVI idiom or similar. When you have additional context to work out whether a function should be virtual, then it becomes easier to avoid mistaking whether it actually is virtual (both when defining it and when using it). I agree that override is nice too.Fisher
@onebyone: Thanks for mentioning NVI, since it got me to go read gotw.ca/publications/mill18.htm. I'm familiar with the template pattern but I'll have to give NVI more consideration before I can decide what I think of it. I do like what he said about destructors being either protected or virtual.Regenerative
Base class destructors, that is. The majority of my classes still have public, non-virtual destructors, because they aren't designed to be polymorphic base classes. Again C# gets it right with "sealed", except that in C++ that would be the default...Fisher
@onebyeone: If I remember correctly, the C++ idiom to enforce "sealed" is to make the destructor private. But the idea of making that the default is interesting. Certainly, it would make planning for inheritance more of conscious decision.Regenerative
Well, that stops anyone destructing the class at all, so you need to provide factory-style create and destroy, and objects can't be placed on the stack by users, which is a pain. There's another trick where you give the class a virtual base, that has a private constructor, and your class is a friend of the virtual base. That has almost the desired semantics (unfortunately you can still inherit, but you can't instantiate the resulting derived class because virtual bases are always constructed by the most-derived class). The downside is it most likely increases the size of the objects.Fisher
Thinking about it, maybe the thing to do is to have the virtual base only present in debug mode. Of course if a client is going to ignore the instructions in the documentation that my class is not intended for use as a base class, they might not care that their code doesn't compile in debug mode. But it will prevent accidents as long as the client uses debug mode at all, and the object bloat shouldn't matter.Fisher
@onebyone: I should have said that that the constructor would be private, not the destructor. Private destructors are an idiom used with reference-counted objects, such as in COM. Anyhow, with a private constructor, you would need a factory method, but I think it could be used to create an object on the stack so long as you had a public copy constructor. The scheme you suggested, with the private friend parent is cleaner, and I'm not sure that the size increase is necessarilly significant. I'd be a bit nervous, though, about code that works only in one type of build.Regenerative
Well, if the object size was significant, then I'd say that the class "works" in both kinds of builds, but as an optimisation it only performs the desired check in debug. That's what asserts do too, so it's not unprecedented, it's just that you wouldn't want to use an assert for this even if you could, because it ought to be checked at compile time.Fisher
@onebyone: That's true. I guess we have to just weigh the negatives on each side. We can talk about what's likely to win out in a typical case, but we can't generalize to all cases.Regenerative
Absolutely - if the likes of me could come up with bulletproof general solutions to this kind of question, then C++0x would have been published years ago :-)Fisher
M
4

Adding virtual does not have a significant impact either way. I tend to prefer it but it's really a subjective issue. However, if you make sure to use the override and sealed keywords in Visual C++, you'll gain a significant improvement in ability to catch errors at compile time.

I include the following lines in my PCH:

#if _MSC_VER >= 1400
#define OVERRIDE override
#define SEALED sealed
#else
#define OVERRIDE
#define SEALED
#endif
Metalepsis answered 3/9, 2009 at 1:47 Comment(4)
Nice - I was unaware that MSVC had this for native C++ code. I may do something similar to what you have (though I suspect I'll get overridden by the team - "that's not C++!").Alyce
Thanks, I've learned something new here! However, I'm tempted to say that I don't want to use non-standard language extensions unless I absolutely have to. It seems that the #define-magic can make the code compile cross-platform but it is likely that it won't restrict errors creeping into it when you work on the code base outside of windows. Hence no vote up or down.Booklover
You can (at least in theory) get rid of your #define magic now. override is now standard (C++11) and you can replace sealed with final (also C++11).Jolandajolanta
Or if you still need to use older versions of Visual Studio, you could rewrite it so that it defaults to defining SEALED to final, but sets it to sealed for said older versions of VS.Certification
O
0

I can think of one disadvantage: When a class member function is not overridden and you declare it virtual, you add an uneccessary entry in the virtual table for that class definition.

Objectionable answered 30/9, 2009 at 16:20 Comment(1)
If the base method has already been overridden you already have that penalty. Marking the overridden function virtual doesn't add anything. It's purely a note to programmers.Kuykendall
B
0

Note: My answer regards C++03 which some of us are still stuck with. C++11 has the override and final keywords as @JustinTime suggests in the comments which should probably be used instead of the following suggestion.

There are plenty of answers already and two contrary opinions that stand out the most. I want to combine what @280Z28 mentioned in his answer with @StevenSudit's opinion and @Abhay's style guidelines.

I disagree with @280Z28 and wouldn't use Microsoft's language extensions unless you are certain that you will only ever use that code on Windows.

But I do like the keywords. So why not just use a #define-d keyword addition for clarity?

#define OVERRIDE
#define SEALED

or

#define OVERRIDE virtual
#define SEALED virtual

The difference being your decision on what you want to happen in the case you outline in your 3rd point.

3 - If, through some error, the virtual were removed from IFoo, all children will still function (CFooSpecialization::DoBar would still override CFooBase::DoBar, rather than simply hiding it).

Though I would argue that it is a programming error so there is no "fix" and you probably shouldn't even bother mitigating it but should ensure it crashes or notifies the programmer in some other way (though I can't think of one right now).

Should you chose the first option and don't like adding #define's then you can just use comments like:

/* override */
/* sealed */

And that should do the job for all cases where you want clarity, because I don't consider the word virtual to be clear enough for what you want it to do.

Booklover answered 17/2, 2014 at 22:40 Comment(10)
Just gotta point out two things: 1) override and sealed go after the parameter list, not at the start. void func() virtual { /* ... */ } isn't a valid function, but void func() override { /* ... */ } is, as long as it overrides a virtual function in a base class. 2) As of C++11, override is an actual specifier in ISO C++ with the same semantics as the Microsoft-specific version, and final is also an ISO C++11 specifier with (to my knowledge) the same semantics as the Microsoft-specific sealed.Certification
@JustinTime Considering that the question was asked in 2009, my answer is for C++03, which some people still use. I'm well aware that C++11 does provide the keywords you mentioned, but I don't think that it yet applies to all of the C++ community. However, your comment does point out that I potentially require a preamble for my answer.Booklover
@JustinTime additionally, where in my answer did you find me suggesting to put the virtual keyword after the parameter list? I really don't understand where your point 1) is coming from...Booklover
That would be the #define OVERRIDE virtual and #define SEALED virtual macros. If you use those definitions, and use OVERRIDE and SEALED as you would override and final or the older Microsoft-specific override and sealed, then the preprocessor will turn, for instance, void func() OVERRIDE { /* ... */ } into void func() virtual { /* ... */ }.Certification
Also, C++11 doesn't add the context-sensitive keyword sealed, it adds final, which appears, as far as I can tell, to have the same semantics as the older Microsoft-specific sealed.Certification
@JustinTime Do you know of any good link about the ins and outs of the semantics of override and final?Stereophotography
@greendiod I believe this one has a good summary: learncpp.com/cpp-tutorial/… This one is also useful: blog.smartbear.com/c-plus-plus/…Certification
@greendiod Basically, what it boils down to is: a function declared override will cause the compiler to give you an error if it doesn't override a virtual function with the same signature; a function declared final will cause the compiler to give you an error if a more derived class tries to override it; and a class declared final will cause the compiler to give you an error if you try to derive from it. Both keywords only have meaning when used as an identifier; they can be used as variable/class/function/etc. names in other contexts, to prevent pre-existing code from breaking.Certification
@JustinTime Yes, using them as you would override and final would be wrong but that's not what I'm suggesting at all. Nor did I recommend using them like you would Microsoft's keywords, in fact I thought I said quite the opposite. The intent was for people to use these keywords as simple style hints to the developer where they are still unavailable. If you're using C++11 I sure hope you're not using these #define macros. Again, this was a long time ago and is intended for C++03. Are you happy with the preamble as it is or do I need to change something else?Booklover
@Booklover Ah, I get ya. I thought you were suggesting that people use those macros in the same position in the function signature as they would override, final, or sealed, not at the start. If they were, defining the macros as virtual would likely cause them problems. And yeah, I believe the preamble is accurate.Certification

© 2022 - 2024 — McMap. All rights reserved.