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<T> and System.Observable<T>. 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<T>, the Observable<T> 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<T> 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<T>. 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<T>). 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<T>.
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<T>. 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<T>. With ReactiveExtensions, the asynchronous composition is very useful and makes IObservable<T> better than Task<IEnumerable<T>> in this case.
public class TutorialController
{
public IObservable<T> DoLongOperation()
{
return Observable.Create(() => ...);
}
}
var results = Observable.Subscribe(p => OnNext(p), e => OnException(e), () => OnCompleted());
IObservable<T> 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.
DoLongOperationsounds like exactly the kind of method that could useTask. This is exactly what asynchronous programming was created for. \$\endgroup\$