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.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.
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<T>). This lets users use
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
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.
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
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.