Is it bad practice to call a virtual function from constructor of a class that is marked final
Asked Answered
W

3

19

Normally calling virtual functions from constructors is considered bad practice, because overridden functions in sub-objects will not be called as the objects have not been constructed yet.

But, Consider the following classes:

class base
{
public:
   base() {}
   ~base() {}

private:
   virtual void startFSM() = 0;
};

class derived final : public base
                    , public fsm_action_interface
{
public:
    derived() : base{}
              , theFSM_{}
    {  startFSM(); }


    /// FSM interface actions

private:
   virtual void startFSM()
   { theFSM_.start(); }

private:
    SomeFSMType theFSM_;
}

In this case class derived is marked as final so no o further sub-objects can exist. Ergo the virtual call will resolve correctly (to the most derived type).

Is it still considered bad practice?

Whitish answered 30/6, 2014 at 17:6 Comment(7)
If it bothers you, move the implementation into a non-virtual function and have both the virtual function and the constructor call that instead.Prochoras
@Whitish is a very informative question, I have no idea a call to virtual functions from a derived consteuctor had different behaviors in the overriden behaviors. Is this behavior in the ISO standard, and realistically will vary from compiler to compiler? Also just a quick note the virtual method is made private and you inherit publicly.Mangonel
What happens if, a year down the line, you decide that derived shouldn't be final after all?Extravagate
re "Normally calling virtual functions from constructors is considered bad practice, because overridden functions in sub-objects will not be called as the objects have not been constructed yet.", if that is so among your fellow students or colleagues, then they need to be educated. AFAIK it is not a common point of view among C++ programmers. But if so, then C++ programmers at large would need to be educated.Waadt
What method in base normally calls startFSM? That seems relevant so we can make a fully educated decision.Nochur
"Normally calling virtual functions from constructors is considered bad practice, because overridden functions in sub-objects will not be called as the objects have not been constructed yet." -- Erm. AFAIK when the body of the constructor is executed, everything has been constructed, not only the sub-objects but also the member objects of the class/struct itself.Smoky
@Jefffrey: i think he's talking about derived classes, and corresponding sub-objects of the total most derived object to-be. essentially, in my interpretation he's saying that the calls are considered bad practice in C++ because they're not unsafe as in Java or C#.Waadt
W
8

Regarding

Normally calling virtual functions from constructors is considered bad practice, because overridden functions in sub-objects will not be called as the objects have not been constructed yet.

That is not the case. Among competent C++ programmers it’s normally not regarded as bad practice to call virtual functions (except pure virtual ones) from constructors, because C++ is designed to handle that well. In contrast to languages like Java and C#, where it might result in a call to a method on an as yet uninitialized derived class sub-object.

Note that the dynamic adjustment of dynamic type has a runtime cost.

In a language oriented towards ultimate efficiency, with "you don't pay for what you don't use" as a main guiding principle, that means that it's an important and very much intentional feature, not an arbitrary choice. It's there for one purpose only. Namely to support those calls.


Regarding

In this case class derived is marked as final so no o further sub-objects can exist. Ergo the virtual call will resolve correctly (to the most derived type).

The C++ standard guarantees that at the time of construction execution for a class T, the dynamic type is T.

Thus there was no problem about resolving to incorrect type, in the first place.


Regarding

Is it still considered bad practice?

It is indeed bad practice to declare a member function virtual in a final class, because that’s meaningless. The “still” is not very meaningful either.

Sorry, I didn't see that the virtual member function was inherited as such.

Best practice for marking a member function as an override or implementation of pure virtual, is to use the keyword override, not to mark it as virtual.

Thus:

void startFSM() override
{ theFSM_.start(); }

This ensures a compilation error if it is not an override/implementation.

Waadt answered 30/6, 2014 at 17:55 Comment(6)
I have to disagree. See Item 9: Never call virtual functions during construction or destruction in Effective C++ by Scott Meyers, and Item 49: Avoid calling virtual functions in constructors and destructors in C++ Coding Standards by Herb Sutter and Andrei Alexandrescu.Bookmaker
@nosid: that sounds like an authority argument. i've disagreed with scott and andrei on many occasions. sometimes they've agreed with me, sometimes not. why not cite the rationale that you find convincing.Waadt
"Among competent C++ programmers it’s normally not regarded as bad practice." I disagree with that sentence, because apparently it is commonly regarded as bad practice -- regardless whether there is a good reason for doing that or not.Bookmaker
He did not pull rank on them at all. He's entirely correctly pointing out that it's an appeal to authority.Gallimaufry
@nosid: Oh. You may be right, far more people use c++ nowadays, with rote memorization of rules replacing understanding of issues. E.g., in the rationale for their rule Herb and Andrei write that virtual functions don't behave virtual inside constructors and destructors, which is technically false and which they of course know is false: it's an oversimplification. It's suitable for rote memorization, rather than understanding the notion of dynamic type. :(Waadt
This is all technically correct but it doesn't, in my mind, address the root concern here: why are you calling a virtual method in a context that specifically inhibits the polymorphic aspect of the call. That's a strong design smell and in many cases can be addressed in an alternate fashion.Nochur
O
9

This would still be considered bad practice as this sort of this almost always indicates bad design. You'd have to comment the heck out of the code to explain why this works in this one case.

T.C.'s comment above reinforces one of the reasons why this is considered bad practice.

What happens if, a year down the line, you decide that derived shouldn't be final after all?

That said, in the example above, the pattern will work without issue. This is because the constructor of the most derived type is the one calling the virtual function. This problem manifests itself when a base class's constructor calls a virtual function that resolves to a subtype's implementation. In C++, such a function won't get called, because during base class construction, such calls will never go to a more derived class than that of the currently executing constructor or destructor. In essence, you end up with behavior you didn't expect.

Edit:

All (correct/non-buggy) C++ implementations have to call the version of the function defined at the level of the hierarchy in the current constructor and no further.

The C++ FAQ Lite covers this in section 23.7 in pretty good detail.

Scott Meyers also weighs in on the general issue of calling virtual functions from constructors and destructors in Effective C++ Item 9

Outdo answered 30/6, 2014 at 17:27 Comment(10)
@bstsr55 Can this issue be implementation dependent depending on the compiler you use, or is it in the ISO standard?Mangonel
This issue is not implementation dependent. I added a comment to clarify that point.Outdo
-1 "This would still be considered bad practice as this sort of this almost always indicates bad design" the call is not bad practice in the first place (but declaring a member function virtual in a final class is meaningless and so is indeed bad practice and bad design).Waadt
@Alf: The final derived class inherited the virtual modifier on that member function... It may be meaningless to declare the member function virtual, because it will be virtual regardless if the base class declared it so, but documenting that it is virtual is very common. Documenting it with the new override keyword is even better of course.Swiftlet
@BenVoigt: thanks, I didn't see that. I would use override instead of virtual to document that it is an implementation. Reading with the expectation of good code...Waadt
@Alf I don't know how to reconcile your comment against Josh Block's Effectve C++ Item 9: Never call virtual functions during construction and destruction. Every C++ programmer that's ever read this book is apparently in the group of C++ programmers at large that "need to be educated"Outdo
@bstar55: your conclusion about "every" does not follow. readers don't need to share the author's opinion. Josh Block does appear to need some education here, if what you report is accurate, yes. So does Herb Schildt. To name but two.Waadt
@Outdo s/Josh Block/Scott Meyers/Extravagate
@Extravagate Whoa, oops. Got my Effective Java and Effective C++ mixed up. Updated to give credit where credit is due. Thanks!!!Outdo
@bstar55: with the 4th edit's link to Scott's writing I found this at the end of the item 9: "Don’t call virtual functions during construction or destruction, because such calls will never go to a more derived class than that of the currently executing constructor or destructor." I.e. because they're safe. That's meaningless. But the earlier "calls won’t do what you think" means that he's writing specifically to programmers who are unable to understand the notion of dynamic type, what we call populism when a politican does it. Argh. :(Waadt
W
8

Regarding

Normally calling virtual functions from constructors is considered bad practice, because overridden functions in sub-objects will not be called as the objects have not been constructed yet.

That is not the case. Among competent C++ programmers it’s normally not regarded as bad practice to call virtual functions (except pure virtual ones) from constructors, because C++ is designed to handle that well. In contrast to languages like Java and C#, where it might result in a call to a method on an as yet uninitialized derived class sub-object.

Note that the dynamic adjustment of dynamic type has a runtime cost.

In a language oriented towards ultimate efficiency, with "you don't pay for what you don't use" as a main guiding principle, that means that it's an important and very much intentional feature, not an arbitrary choice. It's there for one purpose only. Namely to support those calls.


Regarding

In this case class derived is marked as final so no o further sub-objects can exist. Ergo the virtual call will resolve correctly (to the most derived type).

The C++ standard guarantees that at the time of construction execution for a class T, the dynamic type is T.

Thus there was no problem about resolving to incorrect type, in the first place.


Regarding

Is it still considered bad practice?

It is indeed bad practice to declare a member function virtual in a final class, because that’s meaningless. The “still” is not very meaningful either.

Sorry, I didn't see that the virtual member function was inherited as such.

Best practice for marking a member function as an override or implementation of pure virtual, is to use the keyword override, not to mark it as virtual.

Thus:

void startFSM() override
{ theFSM_.start(); }

This ensures a compilation error if it is not an override/implementation.

Waadt answered 30/6, 2014 at 17:55 Comment(6)
I have to disagree. See Item 9: Never call virtual functions during construction or destruction in Effective C++ by Scott Meyers, and Item 49: Avoid calling virtual functions in constructors and destructors in C++ Coding Standards by Herb Sutter and Andrei Alexandrescu.Bookmaker
@nosid: that sounds like an authority argument. i've disagreed with scott and andrei on many occasions. sometimes they've agreed with me, sometimes not. why not cite the rationale that you find convincing.Waadt
"Among competent C++ programmers it’s normally not regarded as bad practice." I disagree with that sentence, because apparently it is commonly regarded as bad practice -- regardless whether there is a good reason for doing that or not.Bookmaker
He did not pull rank on them at all. He's entirely correctly pointing out that it's an appeal to authority.Gallimaufry
@nosid: Oh. You may be right, far more people use c++ nowadays, with rote memorization of rules replacing understanding of issues. E.g., in the rationale for their rule Herb and Andrei write that virtual functions don't behave virtual inside constructors and destructors, which is technically false and which they of course know is false: it's an oversimplification. It's suitable for rote memorization, rather than understanding the notion of dynamic type. :(Waadt
This is all technically correct but it doesn't, in my mind, address the root concern here: why are you calling a virtual method in a context that specifically inhibits the polymorphic aspect of the call. That's a strong design smell and in many cases can be addressed in an alternate fashion.Nochur
O
0

It can work, but why does startFSM() need to be virtual? In no case do you actually want to actually call anything but derived::startFSM(), so why have any dynamic binding at all? If you want to have it call the same thing as a dynamically binded method, make another non-virtual function called startFSM_impl() and have both the constructor and startFSM() call it instead.

Always prefer non-virtual to virtual if you can help it.

Offload answered 30/6, 2014 at 20:25 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.