Events vs Classes to be overridden: your preference in RhinoCommon?

Rhino Developer
1

Hi all

@stevebaer recently had a go at adding new virtual methods to Rhino.UI.MouseCallback (more info).

He considered adding this as .Net events and we ended up having a conversation about pros and cons of exposing an event-based SDK. In RhinoCommon, we already have some places that expose an event-based pattern, and some other places that show derived classes.

Examples are, for the events side:

  • RhinoApp.Idle, RhinoApp.Initialized
  • DisplayPipeline.CalculateBoundingBox, DisplayPipeline.DrawXXX

On the override side, today we have at least these two classes:

  • Rhino.UI.MouseCallback
  • Rhino.Display.DisplayConduit

So, do you, as a user of these classes, have preferences for a pattern over the other? Do you think McNeel should standardize on a specific pattern, maybe for consistency or for usability?

Thanks for your opinion,

Giulio

Giulio Piacentino
for Robert McNeel & Associates
giulio@mcneel.com

2

@Mitch, @jordy1989 or @djordje, maybe some of you guys might have an opinion regarding this?

3

To be hones I like working with events more than with override. This is because its very obvious how to use. Create a handdler or dispose the handdler. With overrides you need to exactly know what to do before you can get it to work.

Example with DisplayConduit:

The current method with override:

Imports Rhino.Display
Imports Rhino.Geometry
Imports System.Drawing
Imports Rhino

Public Class PatchConduitLeft

    Inherits DisplayConduit

    Private _bbox As BoundingBox

    Protected Overrides Sub CalculateBoundingBox(ByVal e As CalculateBoundingBoxEventArgs)
        MyBase.CalculateBoundingBox(e)
        e.BoundingBox.Union(_bbox)
    End Sub

    Public Sub New()
                _bbox = Brep.GetBoundingBox(True)
    End Sub

    Protected Overrides Sub PostDrawObjects(ByVal e As DrawEventArgs)
            e.Display.PushCullFaceMode(CullFaceMode.DrawFrontAndBack)
            MyBase.PostDrawObjects(e)
            Dim vp = e.Display.Viewport
            e.Display.EnableLighting(True)
            e.Display.DrawBrepShaded(Brep, Mat)
        End If
    End Sub

End Class

How you probably do it with an event:

//somewhere add the handdler like Addhanddler Rhino.Display.DisplayConduit, AddressOf DisplaySub

Private Sub DisplaySub (ByVal sender As Object, ByVal e As Rhino.Display.DisplayConduitEventArgs)
            e.Display.PushCullFaceMode(CullFaceMode.DrawFrontAndBack)
            e.Display.EnableLighting(True)
            e.Display.DrawBrepShaded(Brep, Mat)
End Sub

2nd example would be clearer because most of the things are handled by events (for me at least). It’s less code and less figuring out how to use it for the Developer. Couple of examples where Events are used for me are:

AddHandler Rhino.Commands.Command.BeginCommand, AddressOf StartOfCommand
AddHandler Rhino.Commands.Command.EndCommand, AddressOf EndOfCommand
AddHandler Rhino.RhinoDoc.CloseDocument, AddressOf InterceptRhinoClose
AddHandler Rhino.RhinoDoc.BeginSaveDocument, AddressOf BeforeRhinoCloses
AddHandler Rhino.Commands.Command.UndoRedo, AddressOf UndoRedoCommand

This way is clearer because everything is then written the same way. If the boundingbox can be calculated automaticly is more easier than have too override it. With DisplayConduit at least I think this could be possible.

I hope my explanation is not that hard to read. I think this is more of a personal thing… but it would be easiest to have one way of doing things :wink:

Also I’m not a programmer from origin. So I dont how a “real” programmer thinks about this stuff

4

It’s really contextual for me. Simple stuff is easier with events, more complex stuff typically calls for overridden classes.

One problem I have with events is that it’s not always clear whether they interfere with garbage collection in unforeseen ways. On the other hand, they are more resilient to SDK breakages than abstract classes or interface implementations.

If I had to pick one or the other, I’d go for classes rather than events, probably because it seems more integral to the .NET core philosophy.

5

I just don’t think it’s possible to do it one way or the other. I think we need both because Events proc everything something happens that trigger them. Overrides are more static. You change it or you won’t. MouseCallBacks are things that are not static so an Event would be more obvious. A DisplayConduit is set and changed when you want it. Not when its procced.

So probably some things need Events while other need overrides.

Events = Procced / triggered
Overrides = Static / YOU change it

6

Thanks for your feedback so far.

Yes I (and I know others) were caught with this more than once.

I think that the .Net core team did not have to add events at all to .Net. But they chose to, so somebody might have liked them after all. : )

You definitely have a point regarding “usage hint”. Although none of the overrides we actually provide today really really work like overrides, in that they would allow the class developer to hide the basic implementation of the method. This would actually be prone to hierarchy conflicts between different plug-ins. The basic implementation of virtual methods to be overridden today is usually empty.

I will tell my opinion regarding overrides: the main issue I have is with the Enabled property, which actually is a complex method that will have to add the class to some garbage-collection-safe list, if the constructor did not do so already. Other than this, I think that they are a useful mechanism that really works similar as events. All in all I would pick events at least for mouse and keyboard, though. Events are equally error prone regarding garbage collection.

7

Here’s my opinion, we need both. There are cases where overridden classes make sense and cases where events make more sense (and cases where INotifyPropertyChanged makes sense) There are also developers who prefer one technique over the other. Sometimes this is driven by the language that the developer chooses to write in.

Overridden classes are good for grouping related functions and it is simple to tell if they are on/off with if some sort of ‘Enabled’ property is provided.

Events make it simple to add a function to an existing class that will be called back when something happens, but they tend to be difficult to tell if they are enabled and even worse many developers forget to remove them when they are not needed.

I think that this conversation stemmed from the fact that we have a MouseCallback class with added functionality, but there are no mouse ‘events’ available. I did this on purpose. I’ve got the code working in RhinoCommon where we could choose to add events to specific instances of RhinoViews instead of as static events that work for all RhinoViews and this is where things get difficult from an SDK perspective. There are cases where instance events make sense and there are cases where static events make sense. I haven’t made the events available since I don’t have a good idea of which types of events to expose. If I expose both static and instance events, then I would also need to make it clear through naming what each event is for. In other words, would a MouseDown event work for just one RhinoView or all RhinoViews? Since MouseCallbacks is a class, I can add some descriptive functions in the future for ‘binding’ to a single RhinoView, multiple RhinoVies, or all RhinoViews…