1

I've got an extension method that raises cancellable events, returning a bool if they're cancelled:

public static bool RaiseCancel<T>(this EventHandler<T> ev, object sender, T e) where T : CancelEventArgs
{
    if (ev == null)
    {
        return false;
    }

    foreach (Delegate del in ev.GetInvocationList())
    {
        try
        {
            ISynchronizeInvoke invoke = del.Target as ISynchronizeInvoke;
            if (invoke != null && invoke.InvokeRequired)
            {
                invoke.Invoke(del, new[] { sender, e });
            }
            else
            {
                del.DynamicInvoke(sender, e);
            }
        }
        catch (TargetInvocationException ex)
        {
            throw ex.InnerException;
        }

        // if (e.Cancel) return true;
    }

    return e.Cancel;
}

However, I can't help thinking it should return immediately when a handler cancels it for the sake of efficiency, rather than continue to call the remaining handlers. As far as I'm aware, no handler of a cancellable event should EVER take any action other than to set the Cancel property to true. That being the case, what's the point asking more handlers to make a decision that's already been made? On the other hand, it seems wrong NOT to call an event handler when it happens if an object is listening for the event.

Should I uncomment that if statement (and replace the return at the end of the method with return false;) or not?

EDIT: I suppose if you're going to continue to call handlers, should I allow handlers themselves to make the decision (i.e., they can have if (e.Cancel) return; at the beginning of the handler) if they want?

Flynn1179
  • 11,925
  • 6
  • 38
  • 74
  • You should also ask if remaining handlers should be invoked if a previous one throws an exception. I would say yes they should. – João Angelo Apr 07 '11 at 09:56
  • I already did. http://stackoverflow.com/questions/3113822/raising-events-in-c-that-ignore-exceptions-raised-by-handlers. This code's still in development, I'll probably replace the throwing of the inner exception with a trace output or something. – Flynn1179 Apr 07 '11 at 10:09

1 Answers1

3

NOTE: what I describe here is only what I think makes sense. It might not be implemented this way in the .NET framework (see João Angelo's comment below)

Take an example: the FormClosing event. If a handler cancels this event, the form is not going to be closed, so it doesn't make sense to notify the other handlers that the form is being closed.

In the more general case, if you call the other handlers after e.Cancel is set to true, you're notifying them of an something that isn't happening anymore...

So in my opinion you should stop invoking the handlers as soon as e.Cancel is set to true.

Thomas Levesque
  • 286,951
  • 70
  • 623
  • 758
  • IIRC, `FormClosing` calls all handlers and a second handler can even cancel a previous cancel by setting `e.Cancel = false;`. – João Angelo Apr 07 '11 at 10:18
  • @João Angelo, actually I didn't check... I only described the behavior that makes sense to me – Thomas Levesque Apr 07 '11 at 10:26
  • Is that not always the case though? What's stopping any subsequent handler setting `e.Cancel` to false? And isn't it a bit antisocial to do that? What if this previous handler really needed that event to be cancelled? – Flynn1179 Apr 07 '11 at 10:29
  • what you described also makes sense to me, but your example can lead someone in error by believing that's exactly what happens. – João Angelo Apr 07 '11 at 10:38
  • @João Angelo, you're right. I edited my answer to make it clear – Thomas Levesque Apr 07 '11 at 11:47