51

If I have the below code, should I just call the Action or should it call Action.Invoke?

public class ClassA
{
  public event Action<string> OnAdd;

  private void SomethingHappened()
  {
    if (OnAdd != null)
     OnAdd("It Happened"); //Should it be OnAdd.Invoke("It Happened") ???????
  }
}

public class ClassB
{

  public ClassB()
  {
    var myClass = new ClassA();
    myClass.OnAdd += Add;
  }

  private void Add(string Input)
  {
    //do something
  }  
}
3
  • 10
    In c# 6 it may end up being more popular to use the new OnAdd?.Invoke("It Happened"); syntax
    – Betty
    Feb 13, 2015 at 8:14
  • 1
    A little more detail to the comment above. That example is using a feature of C# 6, the Null-Conditional Operator. The ? operator prevents the calling of Invote method if OnAdd is null thus preventing an exception. This allows the writing of code that foregoes the explicit null checking show in the answers below. See msdn.microsoft.com/en-us/magazine/dn802602.aspx for more detail.
    – Mark
    Feb 28, 2017 at 10:27
  • 1
    Some comparison here: jacksondunstan.com/articles/3283 . Jan 11, 2018 at 11:54

5 Answers 5

68

The two are equivalent, the compiler converts OnAdd("It Happened"); into OnAdd.Invoke("It Happened"); for you.

I guess it's a matter of preference, however I personally prefer the terser form.

As an aside, it is generally preferable to take a local copy of a class level delegate before invoking it to avoid a race condition whereby OnAdd is not null at the time that it is checked, but is at the time that it is invoked:

private void SomethingHappened()
{
  Action<string> local = OnAdd;
  if (local != null)
  {
    local("It Happened");
  }
}
2
  • 4
    I don't see the difference in the race example, sorry
    – Jon
    Feb 13, 2012 at 11:18
  • 4
    @Jon: Think about if there are two threads involved, and one thread sets OnAdd to null after the other thread has tested for it being null but before invoking it... in your code, you'd end up with a NullReferenceException
    – Jon Skeet
    Feb 13, 2012 at 11:22
26

Something I noticed on this with the latest C# 6 release as it may encourage Invoke to be used more and thought I'd add it to this old question in case it helps someone:

"Old" way:

Action<string> doSomething = null; // or not null
if (doSomething != null)
    doSomething("test");

Possible pragmatic way (similar to empty event delegate pattern):

Action<string> doSomethingPragmatic = s => { }; // empty - might be overwritten later
doSomethingPragmatic("test");

C# 6:

Action<string> doSomethingCs6 = null; // or not null
doSomethingCs6?.Invoke("test");

// Not valid C#:
// doSomethingCs6?("test")
// doSomethingCs6?.("test")
1
  • 2
    ReSharper encourages this new C# 6 style. Oct 6, 2016 at 15:09
19

The two constructs are perfectly equivalent.

OnAdd("It Happened");

is just syntactic sugar. Behind the scenes the compiler emits a call to Action<T>.Invoke in the resulting MSIL. So use the one that's more readable to you (for me OnAdd("It Happened"); is readable enough).

7

They're exactly equivalent unless you run into a very strange bug around anonymous functions.

Personally I typically use the shortcut form, but just occasionally it ends up being more readable to explicitly call Invoke. For example, you might have:

if (callAsync)
{
    var result = foo.BeginInvoke(...);
    // ...
}
else
{
    foo.Invoke(...);
    // ...
}

Here the explicit use of Invoke is useful for symmetry.

See section 15.4 of the C# 4 spec for more details of delegate invocation, although it doesn't explicitly specify it in terms of calling the Invoke method.

2

I prefer to use Invoke() since this allows me to handle a possible null reference with myAction?.Invoke().

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.