Self-Testing delegates: avoid checking for null before invocation?

Question

Is there any smart way to avoid the verbosity of testing the nullity on an event before calling it in a generic way ? It looks obvious that if I call a delegate, I want it to be assigned.
(If I Really want/need to test its nullity I could eventually do it explicitly, but putting this test systematically is kind of tedious and verbose.)

public delegate void ResetTradesDelegate();
public ResetTradesDelegate ResetTradesEvents;

public void OnSessionRxAdmMessage(IVfxFixAppSession session, FixMessage msg)
{    
    if (ResetTradesEvent != null)  //<-- Is there "any" a way not to write this test explicitly for each delegate ?
       ResetTradesEvent();
}

I always copy it before testing (in case subscriptions change between check and call), however it's my understanding the test is necessary. — Brad Christie, Jul 12 '12 at 20:12
One approach some people take is to add a no-op handler for events in the object's constructor. It's not particularly elegant, but it does avoid the need for a null check. — dlev, Jul 12 '12 at 20:12
I don't think this an issue of a `delegate`, but rather that of an `event` (which uses a MulticastDelegate?) and how they are implemented. Here if you assign a "dummy" delegate object (to the *non-event* member) then no check is required and it is *no different* than any other value that can be null .. — , Jul 12 '12 at 20:13
I agree with @dlev, but in my opinion it's "better" to check if something isn't null, before using it — Michiel van Vaardegem, Jul 12 '12 at 20:18
I'd say just go for the no-op handler dlev mentioned. No matter how you write the check, it won't be 100% safe anyway. — Alan, Jul 12 '12 at 20:42
Related: [Is there a downside to adding an anonymous empty delegate on event declaration?](http://stackoverflow.com/questions/170907/is-there-a-downside-to-adding-an-anonymous-empty-delegate-on-event-declaration) — jrh, Feb 15 '17 at 12:37

Pete Baughman · Accepted Answer · 2013-12-25T04:29:57.267

15

public event EventHandler NoDataEventHandler = delegate{};

Declaring an event in this way means it will never be null. It will always, at a minimum, have a single no-op event handler hooked up.

In your case, probably

public event ResetTradesDelegate ResetTradesEvents = delegate{};

Firing an event is always going to have a race condition associated with it. You're either going to risk trying to call a delegate when it's null, or calling a delegate after the event has been unhooked. Eric Lippert wrote a pretty comprehensive post on this topic here. The technique above still suffers from the second type of race condition, so the event handlers need to be robust to being called after the event has been unhooked.

edited Dec 25 '13 at 04:29

answered Jul 12 '12 at 20:30

Pete Baughman

2,996
2
19
33

1

This works *until* you add an event and then remove it (so there are 0 wired events). Now the event delegate is `null` again .. because I have been affected by this issue before (a `null` where not-null was expected), I would recommend *against* using this approach .. – Jul 12 '12 at 22:11
8

I have just written a test program with `public event EventHandler ValueChanged = delegate { };` that subscribes to the event `obj.ValueChanged += new EventHandler(obj_ValueChanged);`, changes a value (thus triggering the event handler), unsubscribes `obj.ValueChanged -= obj_ValueChanged;`, changes the value again (not triggering the event), but without any null reference exceptions. Adding an event means there will be 2 event handlers hooked up. Removing that event will put you back to 1 (the no-op delegate). Are you 100% sure you weren't doing something else? – Pete Baughman Jul 12 '12 at 22:38

score 4 · Answer 2 · answered Jul 12 '12 at 20:39

4

static void CallIfNotNull(this Action action)
{
 if (action != null) action();
}

As an extension method, this is quite convenient to use.

answered Jul 12 '12 at 20:39

usr

168,620
35
240
369

This depends on the semantics of extension methods. If `this Action action` makes a copy of the value of the reference to the `Action`, then yes this works. Otherwise, you could still get a `NullReferenceException` if action was modified by another thread. – Mike Bailey Jul 12 '12 at 20:56
@MikeBantegui, parameters behave like locals. They are copies. – usr Jul 12 '12 at 21:40
That's actually quite interesting. So modifying the values of a struct in an extension method does *not* do what you would expect it to. – Mike Bailey Jul 12 '12 at 21:45
+1 Unfortunately, it is tedious to expand this over Func and different signatures .. also, for `event` if there is multi-threading at all, I believe the general advice is to acquire a lock while accessing the backing MulticastDelegate .. not sure how the implicit conversion to Action affects this. – Jul 12 '12 at 22:09
@pst, this pattern is exactly what you should do in case of concurrent access. The parameter is a thread-safe snapshot of the event's value. This code is thread-safe. Locks are not required because delegates are immutable. – usr Jul 12 '12 at 22:16
@usr Hmm, I could only convert an event delegate to Action via `new Action(eventDelegate)` (but not an implicit) but this *throws an exception* if the event delegate evaluates to null ..? In the case of `new Action(..)` it does appear a copy of the delegate list is made, as advertised. – Jul 12 '12 at 22:30
If you want this function to work on arbitrary delegate types (and I recommend that you just use Action everywhere, even on events) you could make the method parameter of type `Delegate` and use Delegate.DynamicInvoke() to call it (at a certain perf hit). – usr Jul 12 '12 at 22:37
What is meant by "..use Action everywhere, even on events"? Have a signature example? – Jul 12 '12 at 22:39
`public event Action SomeName;` – usr Jul 12 '12 at 23:00

score 2 · Answer 3 · answered Jul 12 '12 at 21:03

You can create your event-handler with an always-subscribed no-op event:

public class MyClass
{
    public MyClass()
    {
        this.Event += (sender, e) => ();
    }

    public event EventHandler Event;

    protected virtual void OnEvent()
    {
        this.Event(this, EventArgs.Empty);
    }
}

However, this requires subscribing your event and will incur a performance penalty as the no-op delegate will still exist in the list of subscribed event handlers.

My preference here is to create a pair of extension methods to invoke any event handler, whilst performing a null-safe check:

public static void Raise(this EventHandler @event, object sender)
{
    if(@event != null)
    {
        @event.Invoke(sender, EventArgs.Empty);
    }
}

public static void Raise<TEventArgs>(
    this EventHandler<TEventArgs> @event,
    object sender,
    TEventArgs args)
    where TEventArgs : EventArgs
{
    if(@event != null)
    {
        @event.Invoke(sender, args);
    }
}

This can then be easily invoked anywhere in your library to safely raise the event:

this.ResetTradesEvent.Raise(this);

It's purely syntactic sugar; you're still doing the check on the delegate. It is however, a nice re-usable way to wrap up this smelly part of the C# language.

Not only does the no-op handler get invoked, but the event will also now always refer to a materialized delegate object (vs. `null`). I believe using `null` was, at least at conception, a design decision to minimize unneeded objects for un-wired events. In any case, still a +1 .. also I think it would be cleaner to just rename `event` then use the keyword-breaker syntax. — , Jul 12 '12 at 22:16
"Keyword-breaker syntax"; I like that phrase. Would you have a more preferable argument name? I thought a little on this when adding it to my own libraries, but couldn't find anything I personally found clearer. — Paul Turner, Jul 13 '12 at 09:03

score 1 · Answer 4 · answered Nov 27 '19 at 17:39

1

Using the null conditional operator keeps the test but is less verbose so works for me. I don't imagine it resolves the race condition that others have mentioned.

ResetTradesEvent?.Invoke();

This is suggested automatically in Visual Studio 2017.

answered Nov 27 '19 at 17:39

Tim MB

4,413
4
38
48

score 0 · Answer 5 · answered Jun 07 '19 at 09:53

0

public static void Call(this Action action)
{
    var safeAction = Interlocked.CompareExchange(ref action, null, null);
    if (safeAction != null)
        safeAction();
}

answered Jun 07 '19 at 09:53

isxaker

8,446
12
60
87

Please add some explanation – Jonathan Willcock Jun 07 '19 at 11:34

Self-Testing delegates: avoid checking for null before invocation?

5 Answers5

Linked