How to differentiate a Virtual Method that should be Augmented vs. Replaced?

[najak3d]

I write two main types of Virtual Methods in a base class (often abstract).

1. Ones meant to be optionally augmented (i.e. MUST or SHOULD be called by overrides)
2. Ones meant to be optionally Replaced, and should NOT be called by the overrides.

What is the correct/best way to differentiate between the two, aside from a comment. And the comment-method is problematic, because the Summary is supposed to tell you "how to use the method" not "how to override it".

So this has always been a bit of conundrum that I've seen, dealing with this ambiguity. I wish there were a keyword to put in the base definition like "protected virtual extended void MethodName()" to signify a method that should be called by the overrides. And then if the override method calls or doesn't call the base method, the compiler will issue a Warning (or error?) - to prevent the mistake.

Example:
We just fixed a long-standing bug, where one certain branch of the code failed to call the Android MainActivity base class method... Since the branch was rare, it rarely happened, but when it happened, it was catastrophic. Just as the compiler warns you when branches omit setting an "out parameter", it would be nice if a virtual method intended to be called by the overrides was likewise enforced as a compiler warning, at minimum.

[CyrusNajmabadi]

it would be nice if a virtual method intended to be called by the overrides was likewise enforced as a compiler warning, at minimum.

This would be a space for analyzers for your domain-specific additional rules you would like to have.

That said, the way i generally handle this is to never make it necessary for the override to call into the base. Indeed, anything that must run is designed to be impossible not to run. The override only exists for the part that is ok to be overridden without the base happening.

In other words. INstead of:

public virtual void Foo()
{
    This.MustRun();
    This.DoesNotHaveTo();
}

I instead write:

public void Foo()
{
    This.MustRun();
    OptionalFooPart();
}

protected virtual void OptionalFooPart()
{
    This.DoesNotHaveTo();
}

Now there's no problem. The subclass can't mess this up, because calling into the base is not necessary.

[theunrepentantgeek]

You are assuming that the author of the base class can anticipate every possible way their class might be extended by any developer anywhere and anywhen.

I'm very sure I can't do that in the general case, and only infrequently in the specific case.

Extensibility of most classes is poorly designed, if it's designed at all.

I literally encounter the problem of classes that are not extensible enough orders of magnitude more often than class that are too extensible.

There are some practitioners who genuinely believe that this is enough of an issue that nothing should be private. Everything should be internal or public. While I disagree with the position (class invariants are too important), I have some sympathy for the position.

Your original problem - failing to call the base method - sounds far more like a deficit of testing more than anything else.

Whether you like it or not, this kind of special case is exactly why analysers were introduced c

[CyrusNajmabadi]

The work-around is:

Not advertised, or well known
Doubles the number of members for each virtual method
Requires a fair amount of extra code (an added method).

Your second and third points are the same here. But as I said, this is not a negative in my book. That said, if it is a negative for you, there is an easy solution which has been available for years: analyzers.

This sort of case is exactly what they were created for and what people already use them for.

So would be nice if C# added some keywords on Virtual method to dictate usage in the overrides.

We are not likely to do this when a completely suitable approach is instead to have an analyzer that adds she attributes to dictate usage in the overrides.

[najak3d]

@CyrusNajmabadi : The issue of Immutability could have also been implemented fully with custom Attributes and analyzers. You could have marked all readonly fields with "[Readonly]" and then use analyzers to determine violations. So why should C# address immutability, since it can already be handled without C# upgrades?

Answer: Because the C# enhancements made enforcement of immutability much easier, and saves code and bugs. Same can be said for Nullability, as well as the majority of C# enhancements.

Your continued line of arguing would lead us to believe that C# 4.0 was fine, and didn't need to be changed.

And:

1. Doubles number of members
2. Added extra code

Are NOT the same Con. Extra code doesn't always (or even usually) result in added class members. The cost of extra code, costs you ONCE locally. The cost of adding members costs the USERS of your class, as well as those maintaining your class, now seeing extra members, to decipher. And introduces new opportunities for defect (derived classes calling your new method, when they should NEVER call it). So these are TWO cons, not just one.

Shouldn't this have already been obvious to you?

[najak3d]

@theunrepentantgeek - The fact that you can't control how others will override your virtuals is exactly why they might not make them virtual, AND they don't want to add more Members to the API, so they don't employ Cyrus's (and my) technique of adding additional members with specialized names. It would be a heck of a lot nicer, if "virtuals" could have influence over the overrides (rules/guidelines for overrides).

Currently Virtual methods have a "summary" comment, but these summaries are about "functionality" and shouldn't be cluttered with "instructions for override" -- and so those overriding usually have no instructions.

Analyzers of this would also be crippled, because the analyzers ALSO do not have a clue as to usage rules for each virtual member, and so won't know when to issue Warnings. Only the author of the base class knows usage intent, but has no feasible facility to communicate this to derived classes.

Also -- these suggested prescriptive keywords are OPTIONAL. Just use them if you KNOW... plus I've suggested keywords to either trigger an Error vs. Warning -- depending upon whether the author of base class wants to communicate a "MUST" vs. a "SHOULD", vs. the absence of keyword means "I have no idea" (which is the current status for ALL protected methods now).

I'd like to make C# better for the default/common case of opening up your class to overrides, with more QA control.

[CyrusNajmabadi]

The issue of Immutability could have also been implemented fully with custom Attributes and analyzers.

Immutability is exactly something I would be ok with an analyzer/SG approach to.

[najak3d]

NEVERMIND -- just looked it up -- "sealed" is ALREADY a thing for preventing the propagation of overrides.

IGNORE THIS QUESTION:
Another question - can a derived class SEAL the virtual status of the method? So that derivations of the derivation CANNOT override the derived override?

[FaustVX]

Yes with override sealed modifier

[najak3d]

For case of making my concerns more clear. Here is an example of work arounds that I've done in the past:

public void Update(float timeStep)
{
     // Core non-optional logic that must run goes here ....
    // .....

    OnUpdate();
} 

virtual protected void OnUpdate() {  }  // does NOTHING unless overridden

Here, those who write derived classes will see BOTH "Update" and "OnUpdate", and may call the wrong one. We've introduced more ways to mess up, and more thinking for the dev to do -- for no good reason, other than a C# deficiency in allowing the base class author to specify usage rules.

What I'd prefer to write is this:

public virtual mustCallFirst void Update(float timeStep)
{
     // Core non-optional logic that must run goes here ....
    // .....
} 

This shorter code snippet produces the SAME effect, forcing the override to call the base method first, and requires no additional class member to enforce it.

This added syntax puts control on usage where it belongs - with the author of the base class, without forcing the addition of "similar named" members to create the same effect.

[najak3d]

I'd add to this proposal, specifiers that should even limit "who can call a method". If for some reason I do still need to create a pattern like this:

(introducing combo term "protected private")

public void Update(float timeStep)
{
      _OnBeforeUpdate();

     // Core non-optional logic that must run goes here ....
    // .....

    _OnAfterUpdate();
} 

virtual protected private void _OnBeforeUpdate() {  }  // does NOTHING unless overridden
virtual protected private void _OnAfterUpdate() {  }  // does NOTHING unless overridden

In the above example, I've introduced a suggested notation of "protected private" -- which means "you can ONLY OVERRIDE this method" -- you cannot CALL this method from a derived class (because it is ONLY intended to be called by the "Update()" method -- no one else.

This highlights another issue with the @CyrusNajmabadi (and my) methodology of creating an extra member to help prevent "misuse" -- we are now opening up another potential defect, in that people can call the wrong method, when they should NEVER be allowed to CALL this other method (only override it).

C# does NOT provide enough "control/prescription on usage" of it's protected methods, nor for it's virtual methods. This is worth serious discussion.

[najak3d]

If just # 1 above were implemented, i.e. "protected private", that would eliminate one of the chief Cons of the current workaround (i.e. creating more methods). At least then, your new methods would not create more API for derived classes.... they would only see this method show up in Intellisense as an option for "override" -- never for calling it.

That seems like an EASY change to C#, no? And provides excellent benefit. I'd think something like this is low-hanging fruit.

[najak3d]

After this helpful discussion, I think I'm going to raise # 1 above to it's own "Idea" thread. As it seems like less-of-a-stretch than the concept of "mustCallFirst" (etc) key words.

[HaloFour]

My point in bringing this up here, is that I don't think this is a niche need - it's universal.

It's universally applicable, sure, but it's a niche use case.

Once you do allow overrides -- you open yourself up to risks of misuse. The ways to handle this in C# are non-elegant, and introduce additional ways to misuse or add defects.

By that thinking every OOP language is inherently defective, yet this is not a common problem, not in C# or any other language. You guide your consumers by the shape of your API, and if that shape necessitates multiple methods then so be it. The language isn't going to add completely new features for uncommon scenarios just to make it slightly easier than all of the existing solutions that have existed for many, many decades.

If just # 1 above were implemented, i.e. "protected private"

private protected is already a legal modifier in C#.

[najak3d]

Thanks @HaloFour for the info on "private protected". I've altered my suggestion to "private virtual" instead -- to mean "Overridable but NOT callable", which IMO, makes sense for that to mean this.

[CyrusNajmabadi]

My point in bringing this up here, is that I don't think this is a niche need - it's universal.

This is a very common idea on a lot of your posts. Even reinforced on your part by stating your surprise that other people are not asking for this (despite thousands of requests asking for other things). You then follow things up with justifications that people don't even realize they're missing this and it truly is necessary.

I truly ask that you examine your biases here. We get tons of feedback about an amazing wealth of topics. Consider truly that if only you are vehemently asking for something that perhaps it really is not universal, and that not everyone else is ignorant and doesn't even realize they're missing this clearly obvious feature that you feel provides so much value.

[najak3d]

I added a new Language Idea here, for y'all to shoot down. Enjoy.

#5885

[CyrusNajmabadi]

for y'all to shoot down.

@najak3d this style of discourse is not really appropriate here. Please be respectful of others here and accept that that may genuinely disagree on merit and not for the sake of disagreement.