One-shot events in C#

I have a class that performs some long-running operation.

This class exposes a Completed event. I'd like clients that use this class to register to get notified ONLY ONCE when the class completes doing its operation.

Would you consider this code appropriate to handle this case:

public class TutorialController
{
    public event Action Complete;

    protected void OnComplete()
    {
        if (Complete != null)
        {
            Complete();
        }

        // Reset to null.
        Complete = null;
    }
}

The usage of "client" code is:

TutorialController controller = new TutorialController();
controller.Complete += ... // Register to get notified.

// Once this is done, it will raise Complete() and clear it.
controller.DoLongOperation();

Due to the current design, the Complete event actually triggers another callback that is passed from the outside:

public void SomeMethod(Action action)
{
    // Code similar to this.
    controller.Complete += () => { action(); };
}

The action passed in is ONLY RELEVANT for the current invocation of the controller, that is - after it would run and complete once, it's no longer needed to notify me of this event (with the current "action" parameter). The next invocation will pass its own action, and so on.

Note that the event sender is an object that is kept around throughout the lifetime of the app, otherwise it would be collected and each time I would get a new copy and register on its event.

Other answers are great, but this is really re-inventing the wheel. What you are looking for is the observer pattern. .NET has great in-built capabilities with observer through System.Observer and System.Observable. You can also get the library Rx-Main through NuGet to gain the ability to compose these, LINQ-style, into operations that can be filtered or exploded or modified as you see fit.

In .NET's Observable, the Observable produces values once it is subscribed to (unless it is a hot observable - that's out of scope of this answer). It will then send an OnComplete 'message' to the subscribed Observer when it has finished. It also has the ability to send an OnError(Exception) notification to the Observer, but only if the exception occurs during the subscription. Exceptions during observation are not implicitly caught.

Before we get onto that, it's worth nothing that you also have Task. It's really worth realizing what you are trying to do here. I feel that putting a Completed event on your class is a code smell as it's breaking encapsulation, and you should really bind the completion to the lifecycle of the method invocation (i.e, make it a return). Semantically speaking, if your method will only produce one result (i.e, "I've finished"), then you should return a Task (or Task). This lets users use async/await.

If your method is going to have a sequence of results before finishing (i.e, it's got an IEnumerable perhaps?) then you should return an IObservable.

Here's how I would lay it out in both cases.

public class TutorialController
{
    public async Task DoLongOperation()
    {
        return Task.Run(() => ....);
    }
}

Task.Factory.StartNew(controller.DoLongOperation()).ContinueWith(() => ...);

Replace the ellipsis with the rest of your DoLongOperation method's code. If your DoLongOperation needs to return a single result, change the return to a Task. By the way, you should only use Task.Run if your long operation is CPU-bound or partially IO/CPU bound.

If your DoLongOperation returns multiple results - for example, it's getting a list of things from an external resource such as a web resource - you should instead return IObservable. With ReactiveExtensions, the asynchronous composition is very useful and makes IObservable better than Task> in this case.

public class TutorialController
{
    public IObservable DoLongOperation()
    {

        return Observable.Create(() => ...);
    }
}

var results = Observable.Subscribe(p => OnNext(p), e => OnException(e), () => OnCompleted());

IObservable leaves the a/synchronous nature of the call up to the caller, so you don't need async on this method.

If you provide the rest of your code for your DoLongOperation I can help convert it to IObservable. But really, the code you have right now seems like a hugeeee code smell. For one, it's mutable, and it's DEFINITELY not thread safe.

By the way one guarantee this code gives you over say @Memleak's answer is that you can invoke Subscribe on a completed Observable and no exceptions will be thrown there is no opportunity for side-effects, which is key in asynchrony. This is infinitely better than having two separate units of functionality in a single function based on whether something has already been Disposed/Completed or not. This adheres to the idea that disposables should fail silently if they have already been disposed of.

As @Jeroen rightly mentioned in chat, .NET events are an implementation of the observer pattern. But Rx's IObservable / TPL's Task allows you to compose asynchronous events and has a notion of completeness - .NET events do not. This is more useful for your use case.

StackExchange Code Review Q#62261, answer score: 20