1

I have a C# class which contains the following:

public event Action<int> UnsupportedMessage;

It also has a method containing

if (template != null)
{
    ...
}
else
{
    if (UnsupportedMessage != null)
    {
        UnsupportedMessage(templateId);
    }
}

Another C# class called HealthCheck contains public void MyMessage(int id).

What's supposed to happen is if template is null, an event should call HealthCheck.MyMessage().

So many things are wrong here. UnsupportedMessage is always null so UnsupportedMessage(templateId); never gets executed. Even is I remove if (UnsupportedMessage != null) UnsupportedMessage(templateId); will throw System.NullReferenceException: 'Object reference not set to an instance of an object.'. Even if it did work, there's no reference to HealthCheck.MyMessage() so I don;t know how it could ever be run.

How do I create an event to do what I need?

Update

I tried changing to UnsupportedMessage?.Invoke(templateId); as mentioned in the below answer and added this in the line above it:

UnsupportedMessage += _handlerClass.UnsupportedMessage;

_handlerClass is initialized by the constructor contains this method:

public void UnsupportedMessage(int value)
{
   ...
}

This initially seemed to work but it seems to be getting called many more times than I expected.

runnerpaul
  • 5,942
  • 8
  • 49
  • 118

1 Answers1

0

It sounds like you're simply missing the event subscription step; you'd presumably need:

objectThatIsGoingToRaiseTheEvent.UnsupportedMessage += healthCheckInstance.MyMessage;

which will make UnsupportedMessage become non-null, and make everything work. You should also change:

if (UnsupportedMessage != null)
{
    UnsupportedMessage(templateId);
}

to either

UnsupportedMessage?.Invoke(templateId);

or (on older C# versions, noting that this is semantically identical)

var handler = UnsupportedMessage;
if (handler != null) { handler(templateId); }

to avoid a race condition that is possible if the last event-handler unsubscribes (-=) between the two reads of UnsupportedMessage in the original version.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • According to [this](https://stackoverflow.com/questions/786383/c-sharp-events-and-thread-safety "C# Events and Thread Safety") question, the check for null gives only a false sense of security. It eliminates a race condition, but not all of them. There is still the irremediable race condition of invoking a handler that has been concurrently unsubscribed. Not invoking a handler that has previously subscribed is (probably) also possible, because the event's backing field is not volatile AFAIK. – Theodor Zoulias Dec 17 '20 at 11:19
  • @TheodorZoulias volatility is a red herring; in *that* kind of race, you can't really say (deterministically) what the exact order was *anyway*, without observing the side-effects; the only "fix" there would be to use a `lock` or similar *over the invoke duration*, which would be very bad re queuing etc; my point: that's not a very interesting race condition, in any reasonable scenario; the one that causes a NRE, by contrast: is very interesting – Marc Gravell Dec 17 '20 at 11:30
  • I think that both race conditions are equally interesting, because they occur together. You can only get a `NullReferenceException` if the last subscriber unsubscribes between the two statements. The counterargument (from the previously linked question) goes: *"You'd be concealing a race condition - it would be better to reveal it! That null exception helps to detect an abuse of your component."* – Theodor Zoulias Dec 17 '20 at 11:37
  • @MarcGravell Thanks. I see what you're saying. I'm not sure where `objectThatIsGoingToRaiseTheEvent.UnsupportedMessage += healthCheckInstance.MyMessage;` would go in my code or what `objectThatIsGoingToRaiseTheEvent` is though. – runnerpaul Dec 17 '20 at 22:00