127

When using myDelegate -= eventHandler ReSharper (version 6) issues:

Delegate subtraction has unpredictable result

The rational behind this is explained by JetBrains here. The explanation makes sense and, after reading it, I'm doubting all my uses of - on delegates.

How then,

  • can I write a non-auto event without making ReSharper grumpy?
  • or, is there a better and/or "correct" way to implement this?
  • or, can I just ignore ReSharper?

Here is simplified code:

public delegate void MyHandler (object sender);

MyHandler _myEvent;

public event MyHandler MyEvent
{
    add
    {
        _myEvent += value;
        DoSomethingElse();
    }
    remove
    {
        _myEvent -= value; // <-- ReSharper warning here
    }
}
1

3 Answers 3

139

Don't be afraid! The first part of ReSharper's warning only applies to removing lists of delegates. In your code, you're always removing a single delegate. The second part talks about ordering of delegates after a duplicate delegate was removed. An event doesn't guarantee an order of execution for its subscribers, so it doesn't really affect you either.

Since the above mechanics can lead to unpredictable results, ReSharper issues a warning whenever it encounters a delegate subtraction operator.

ReSharper is issuing this warning because multicast delegate subtraction can have gotchas, it isn't condemning that language feature entirely. Luckily those gotchas are in fringe cases and you are unlikely to encounter them if you're just instrumenting simple events. There is no better way to implement your own add/remove handlers, you just gotta take notice.

I'd suggest downgrading ReSharper's warning level for that message to "Hint" so that you don't get desensitized to their warnings, which are usually useful.

10
  • 72
    I think it's bad of R# to call the results "unpredictable". They're very clearly specified. "Not what the user might predict" isn't in any way the same thing as "unpredictable". (It's also inaccurate to say that the .NET framework defines overloads - it's baked into the C# compiler. Delegate does not overload + and -.)
    – Jon Skeet
    Jun 24, 2012 at 18:44
  • 6
    @Jon: I agree. I guess everyone got used to the high bar Microsoft set for itself. The level of polish being as high as it is, with so many things in the .NET world that make you "fall into the pit of success", encountering a language feature that is a mere brisk walk next to the pit where there is a chance you may miss it is considered by some to be jarring and warrants a sign saying PIT OF SUCCESS IS THAT WAY --->. Jun 24, 2012 at 20:01
  • 5
    @AllonGuralnek: On the other hand, when was the last time you heard of someone actually having a problem due to this?
    – Jon Skeet
    Jun 24, 2012 at 20:05
  • 6
    @Jon: Heard of a problem with it? I didn't even know about this behavior before this question was posted. Jun 24, 2012 at 20:29
  • 3
    It's curious that R# would warn about delegate subtraction, but not warn about the common implementations of events which have the exact same issue. The core problem is that .net uses a single Delegate.Combine which "flattens" multicast delegates, so if it's given delegates [X,Y] and Z, it can't tell whether the result should be [X,Y,Z] or [[X,Y],Z] (the latter delegate holding the [X,Y] delegate as its Target, and that delegate's Invoke method as its Method).
    – supercat
    Feb 1, 2013 at 23:02
30

You should not directly use delegates to sum or subtract. Instead your field

MyHandler _myEvent;

Should be instead declared as an event as well. This will solve the problem without risking your solution and still have the benefit of event usage.

event MyHandler _myEvent;

Usage of delegate sum or subtract is dangerous because you can lose events when simply assigning the delegate (as per declaration, the developer will not directly infer this is a Multicast delegate as when it is declared as an event). Just to exemplify, if the property mentioned on this question was not flagged as an event, the code below will case the two first assignments to be LOST, because someone simply assigned to the delegate (which is also valid!).

myObject.MyEvent += Method1; 
myObject.MyEvent += Method2;
myObject.MyEvent = Method3;

When assigning Method3, I completely lost the two initial subscriptions. Event usage will avoid this problem and at same time remove the ReSharper warning.

1
  • I never thought of doing it this way, but it does make sense to protect the event delegate usage as long as you keep that underlying event private. This however doesn't work well when there are more specific thread synchronization that must happen in the add/remove handlers such as multiple event subscriptions or sub-subscriptions that need to be tracked as well. However, in any case it removes the warnings.
    – Jeremy
    Dec 1, 2017 at 13:14
-17

set it to = null instead of using -=

4
  • 5
    the remove method of an event should not remove all handlers, but rather the handler that was requested to be removed.
    – Servy
    Feb 23, 2015 at 17:02
  • if he's only added one then if he only removes one in effect he's removed them all. I'm not saying use it in all cases just for this specific resharper message. Feb 23, 2015 at 22:06
  • 6
    But you don't know that the delegate is always removing the only item on the invocation list. This is making the resharper message go away by turning correct working code into incorrect broken code that will, in certain circumstances, coincidentally work, but that will break in unusual and hard to diagnose ways in many cases.
    – Servy
    Feb 23, 2015 at 22:08
  • I had to upvote this because -17 is too harsh of a penalty for someone taking the time to write a "potential" answer. +1 for not being passive, even if you were incorrect.
    – John C
    Feb 6, 2020 at 14:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.