Yesterday I posted an observation on Google+, lamenting the lack of a compiler warning when code in an overridden method failed to call any inherited implementation. This simple oversight in an AfterConstruction override in a situation where the observed bug that arose (a memory leak) could just as easily have been the consequence of a more complex error on my part, caused me to spend a significant amount of time, hunting down the wrong bug.
Eric Grange suggested that AfterConstruction and BeforeDestruction was best forgotten. This was an interesting thought, but when I considered it I concluded that I had to disagree. Not only would it not help me avoid my problem had I ignored them, but doing so would merely have created new opportunities to introduce such time-wasting bugs.
AfterConstruction and BeforeDestruction fulfil a genuine need imho. Most especially I have found them of immense utility when creating frameworks. In a framework I find I often have a base class (or classes) from which further classes are intended to be derived, with (potentially) overridden constructors. Also quite often, the base class never-the-less requires some internal initialisation to reliably occur only after any overridden construction has been completed.
In the specific case that led to my initial post, the base class in question contained an interface reference counting bug fix I originally developed long before it was fixed in the VCL. Actually, it addresses a duo of bugs, one of which manifests during construction and the other during destruction. From memory (my Windows VM is not currently running so I can’t check the VCL source to be certain), the VCL now addresses the constructor side of things but not the destructor.
In any event, my base class incorporates a NewInstance override (effectively BeforeConstruction) and an accompanying AfterConstruction implementation that operated to ensure a positive reference count during the execution of those constructors. This addresses the situation where an object references itself using an interface during construction (e.g. passing a reference to itself to some other object which then discards it) resulting in the reference count “bouncing”, falling back to zero and thus destroying itself. A similar mechanism using BeforeDestruction ensures that “double-destruction” doesn’t occur if self is reference as an interface during destructor execution).
If AfterConstruction and BeforeDestruction didn’t exist (or you ignored them) then in such circumstances you would simply have to contrive your own exactly equivalent mechanism. This mechanism would still – by necessity – rely on virtual methods and overrides and so as far as I can see will still be vulnerable to the omission of calls to the inherited implementation.
But worse, without it being implemented in the base framework further steps would most likely be required to ensure that the alternate mechanism was correctly invoked. As far as I can see, ignoring the provided core mechanism and “rolling your own” would only compound the problem, increasing the potential to introduce a mistake rather than reducing it.
Setting aside the rights or wrongs of an AfterConstruction or BeforeDestruction mechanism in the specific, in the general case overriding a method and then neglecting to call “inherited” is a sufficiently rare occurrence that it would be worth warning about imho.
Furthermore, the compiler directive(s) required to then silence that warning would provide a self documenting and clear signal that the omission is indeed intentional, expected and indeed required (in those rare cases that it genuinely is).
-
Actually, no you really don’t need them, think out of the box here.
The VCL uses them to manage state f.i., which is then “if”-checked to prevent events getting potentially triggered during the destruction… if you needed a big signpost that screams “design issue”, look no further.
There are two different concepts really that are mishmashed and AfterConstruction/BeforeDestruction kludge their way to re-separate them: clean up the design by separating construction (memory management really) from initialization (putting values), and do similarly for destruction.
Here is your “equivalent mechanism”, except it’s NOT equivalent, as it cleaned up your design, and rather than working around a design issue of mishmashed concepts, you now have a design feature, that eliminates the need for magic state-dependent switches, grants your classes safe and simple pooling/streaming capability, facilitates DI & AOP, etc.The refcounting use of AfterConstruction is a hack to plug reference-counted memory management at a non-root object level of the class hierarchy, while making way for reference-counted classes that are not reference-counted… And that last kludge is the root of the “need”: without it, the refcounting could simply have taken place at the call site (as usual). And you “need” refcounted classes that aren’t refcounted in Delphi if you want to use interfaces on VCL classes, because interfaces are hard-tied to refcounting… another mishmash between an OO abstraction concept and a memory management technique.
Last use of AfterConstruction is a dubious convenience use for TThread, when CreateSuspend is false… not exactly and earth shattering use scenario either, first because it encourages mixing construction & initialization, second because at most, it saves one explicit “Start” at the expense of having an implicit boolean passed… doesn’t really make for more explicit code, does it?
So all in all, there just isn’t one non-kludgy reason to have those methods in Delphi.
-
The problem is that calling the inherited implementation is not mandatory in OOP. Although often it makes sense, there are implementations where a derived object may implement a very different behaviour that does not require a call to the parent one. Or for some reason the parent method may just raise an exception when called.
A warning could be “too much”, maybe an “hint” could be better, but there is not really nothing wrong “per se” avoiding an “inherited” call.
I got the habit of always starting an overridden method adding the proper “inherited” call, but that’s just a best practice. -
Eric, it’s all well and good “thinking out of the box”, but in reality you still have to work *IN* the box you are given.
Delphi has custom, virtual constructors. If you implement a base class that requires post-constructor initialisation then it is no good coding your base class sanitarily when people can derive from your class and not follow your own ideal model. Sure, you can say “it’s their own fault” when it doesn’t work, but in practice you know that isn’t how people think.
Similarly, and specifically, the way reference counting is implemented in Delphi is *IN THE BOX*. Think outside the box all you like, you cannot change what the compiler is producing, you simply have to work with it.
To get the Delphi you appear to want, you must first invent a time machine.
But all of that pipe dreaming is completely irrelevant anyway. Your objections to using AfterConstruction may have intellectual value and purity on their side, but have nothing what-so-ever to do with the original problem (I perhaps should have left it simply at that in the first place rather than appearing to engage in a debate on that precise subject
).The original problem is far more general: simply an accidentally omitted call to the inherited implementation of an overridden virtual method. It just so happened that the specific method in this case was AfterConstruction, but the [no doubt interesting] debate over that is a side-show to the fundamental problem I was posting about.
-
LDS, yes I know there are sometimes valid reasons for not calling inherited (and in older versions of Delphi doing so could actually be a mistake – remember when calling inherited on the first override of an inherited abstract method used to raise an EAbstract error ?), but these are very few and far between.
Maybe a hint would be a better idea than a warning and maybe some new, specific compiler directive that could be declared in the method body to suppress that hint/warning would be better than having to disable hints/warnings entirely in or around the entire method or it’s body.
I am not hung up on the particular solution to the problem, but I do think it is a problem worth giving a little time to thinking about .
This isn’t the first time it has caught me out – perhaps the 2nd or 3rd, I don’t recall precisely – but this occasion was the most expensive in terms of red herrings and wild geese I ended up chasing as a result of the symptoms of the bug being so easily confused with an error in my framework relationships.
-
“If you implement a base class that requires post-constructor initialisation”
The only initialization that should happen post-construction in a good design is one that is context-dependent (ie. requires passing context-related parameters, references, etc.), and for those AfterConstruction is not appropriate (doesn’t accept context).
So you will never see a good design that “requires” such a context-independent post-constructor initialization…
“but have nothing what-so-ever to do with the original problem”
I strongly disagree here.
There are two cases in which calling the inherited constructor warrants a warning in a consistent fashion, that’s in a constructor/destructor. Delphi lacks that, sure, but even if it had one, it wouldn’t have helped you since you were using AfterConstruction, which being a kludge/hack is the last place where you would went it (whenever you’re using it, you know you’re doing something dirty in the first place).
Now, if you had complained that Delphi should warn about missing inherited calls (or multiple inherited calls btw) in the constructor or destructor, I would have agreed, but in AfterConstruction? No, that thing is a place for dirty hacks, it’s worth a warning on its own.
And I don’t agree about warning about inherited calls on overridden methods in general, because it wouldn’t make any sense half the time (think Paint, Walk, Quack, etc. actually it only makes sense if the subclass implements only a slight variation or adds to an already implemenetd behavior, but doesn’t define the behavior)
-
Eric: “The only initialization that should happen post-construction in a good design is one that is context-dependent ”
The arrogance of such unilateral statements of “rightness” is staggering. It denies the possibility that there may be requirements beyond what you consider able to be met by your notion of a “pure” design.
In the case of a (domain specific) framework I am currently working on I have a base class which is a container for a variety of objects. However, that base class itself has further classes derived from it. It is the derived classes that are responsible for populating the container. Once the container class is populated – and only after that has taken place – there are processes in the base class that must then take place, and these processes themselves may be extended by the derived container classes.
“Strongly disagree” all you like, but if you would just take your constructor/AfterConstruction blinkers off for a moment (that process I just described is NOT part of the constructor process), you will see that that specific bone that you cannot let go of is just one example of the general case of an extension point in a class that is designed in the base class to be reliably and predictably followed by the execution of a further extension point.
The argument over whether or not designing such behaviour into a constructor (with AfterConstruction providing the follow-on extension point) is completely and utterly irrelevant to the general question of whether the compiler should provide hints/warnings if you fail to invoke the inherited behaviour of an overridden, virtual method.
Those occasions when you consider those warning wouldn’t make sense would be FAR less than “half the time”. On those relatively rare occasions that the warning would be undesirable there would be a partner mechanism for indicating this IN THE CODE, so that you could see the deliberate intent in NOT calling inherited. If you are simply ideologically opposed to the idea of such a warning you would of course be able to just disable that warning in your project settings, like all the other warnings that may or may not be relevant to your project (or offend your purist sensibilities
).Having said that, given that you cannot change the COM-style reference counting behaviour required to be supported in Delphi, I would love to see your solution to the potential constructor suiciding interfaced object that completely avoids the need for AfterConstruction or any other similar (but equally reliable, predictable and dependable) mechanism. Think outside the box all you like, but your solution must still fit *inside* the box you already have: Invention of time machines, reliance on arbitrary rules imposed on developers using your base class which cannot be enforced by the compiler or framework constraints, disregarding inconveniences such as the original requirements and purpose of the reference counting mechanism and it’s COM origins are all of course not allowed.
-
I agree, a compiler hint would be helpful. Perhaps a specific hint, which can be turned off.
Also agree with the need for AfterConstruction()/BeforeDestruction() in some circumstances.
Comments are now closed.
Loading ...
DelphiFeeds
7 comments