Follow-up on the OnXxx anti-pattern

[Started updating the previous entry, but the reply is chunky enough to go in its own post]

One person on the dotnet-clr mailing list has highlighted, quite accurately, that the RaiseXxxEvent method should really take EventArgs as a parameter instead of a string to ensure it can be swapped with a class inheriting from EventArgs. If you want that kind of extensibility, then yes, put an EventArgs as a parameter. I should update the sample.

A second person in the comments vehemently disagree with my point, so let's go for a simple rebuttal, as I do not believe the arguments to be accurate.

The first part is that the pattern, to be deemed acceptable, should be OnXxx(object src, EventArgs ea).  I have nothing else to say but to point with every single of my examples as none of them seems to "get" the right way of declaring OnXxx. As for this way to be the right way, I've not found any other reference to support Peter's argument.

The Design Guidelines for Developing Class Libraries does mention the void delegateType(object source, eventargs e) signature for event handlers, not for event raisers. As for event raisers, here's a big quote from the same source.

Do use a protected virtual method to raise each event. This is applicable only to non-static events on unsealed classes, not to structures, sealed classes, or static events.

Complying with this guideline allows derived classes to handle a base class event by overriding the protected method. The name of the protected virtual (Overridable in Visual Basic) method should be the same as the event name prefixed with On. For example, the protected virtual method for an event named "TimeChanged" is named "OnTimeChanged".

Important Note:

Derived classes that override the protected virtual method are not required to call the base class implementation. The base class must continue to work correctly even if its implementation is not called.

Do use a parameter that is typed as the event argument class to the protected method that raises an event. The parameter should be named e.

Let's see:

  • protected virtual method for non-static events on unsealed classes: Checked
  • the virtual method *should* be prefixed by On. This is non normative, so I'm still within the guideline: Checked
  • The last bit is quite clear: to be within the guideline, the raise method would be Raise(EventArgs e), and not take an object as the first argument: Checked (after having updated the previous post)

Indeed, the second part talks about passing a string instead of an EventArg, this came from typing quickly an example rather than the focus of the pattern, but point taken and resolved.

The third part is interesting. The reader is oblivious to the fact that the event raising method is called RaiseXxxEvent. Missing the italics is an easy mistake to make so let's move on. Apparently OnClear is a rat hole because it doesn't follow the right pattern (neither do the others as said before, and worst, neither does the design guideline...) OnClear with any other argument can mean anything but OnClear(object, EventArgs) can only mean one thing. So in this case, using On can have several meanings and it's fine, but in the case of Raise, you may never know if someone cannot confuse RaiseTheMountain and RaiseGoingUpEvent, so it's not acceptable. I'll assume the whole diatribe is based on the EventArgs confusion and the use of a string, and the missing Event suffix, so I think the point is clear.

So I'll summarize the argument I made extensively: OnXxx is implemented widely but implemented widely wrongly across mscorlib, asp.net, winforms and WPF. It has a double meaning as either raising (as per the design guidelines) or handling (as per people's use of it). The examples in the previous post show that without . Furthermore, the asp.net language use OnXxx to autowire events to event handlers, just like html events.

The confusion is enough that I choose to split the pattern in two methods with clear intent: HandleXEvent to handle an event, RaiseXEvent to raise an event. And even by doing so, I still adhere to the design guidelines issued by Microsoft. If you find this more confusing than OnXxx, I'm just lost for words.

Ads

Comment