Reputation: 12855
Consider the following code:
class A
{
private int _workId;
public void DoWork()
{
_workId = new Random().Next(100);
Console.WriteLine("Starting a long process ID: " + _workId);
LongProcess longProcess = new LongProcess();
longProcess.StartWork();
longProcess.OnWorkMiddle += OnLongProcessWorkMiddle;
Console.WriteLine("At end of long process: " + _workId);
longProcess.OnWorkMiddle -= OnLongProcessWorkMiddle;
}
private void OnLongProcessWorkMiddle(object sender, EventArgs e)
{
Console.WriteLine("In middle of long process: " + _workId);
}
}
The following code is problematic in my eyes for a few reasons:
Console.WriteLine("In middle of long process: " + _workId);
to appear next to Console.WriteLine("At end of long process: " + _workId);
, and yet, they both appear in separated functions._workId
should by all merits a function inside DoWork, however, I am forced to make it a field functions to be able to access that information from inside the callback. Every additional parameter or return value would also become a field member.There are a few solutions that could reduce the amount of problems in this code:
Encapsulating the function and its callback in a different class
One of the issues I've described above were field members that aren't necessarily related to the owner class as a whole, but instead specific to the function DoWork and its callbacks. By encapsulating the function in its own class, we remove that unnecessary clutter and group it in its own logical group. The downside is that it requires a creation of a class from what, by merit, should've been a function. It also doesn't solve issue #1.
Using anonymous delegates to keep in a single function
We could rewrite the code like so:
public void DoWork()
{
var workId = new Random().Next(100);
EventHandler onLongProcessWorkMiddle = delegate { Console.WriteLine("In middle of long process: " + workId); };
Console.WriteLine("Starting a long process ID: " + workId);
LongProcess longProcess = new LongProcess();
longProcess.StartWork();
longProcess.OnWorkMiddle += onLongProcessWorkMiddle;
Console.WriteLine("At end of long process: " + workId);
longProcess.OnWorkMiddle -= onLongProcessWorkMiddle;
}
Now we no longer the need of any field members and we can keep everything nicely in a single function. However, readability-wise I find the code rather lacking as, the code (or rather its delegate) that is executed later on, appears on top.
Using a semi-generic helper class
In this solution I've written a semi-generic helper class to sync the execution. It allows me to write the code in the following way:
public void DoWork()
{
var workId = new Random().Next(100);
LongProcess longProcess = new LongProcess();
var syncr = new EventSyncr();
syncr.Sync(
delegate { longProcess.OnWorkMiddle += syncr.EventCallback; },
delegate { longProcess.StartWork(); },
delegate { Console.WriteLine("In middle of long process: " + workId); },
delegate { longProcess.OnWorkMiddle -= syncr.EventCallback; });
Console.WriteLine("At end of long process: " + workId);
}
And here is the code for the helper class:
/// <summary>
/// Used to synchronize code that waits for a callback from a function.
/// </summary>
class EventSyncr
{
readonly AutoResetEvent _eventCallbackEvent = new AutoResetEvent(false);
private Action _actionAtEvent;
/// <summary>
/// Executes the syncing process. This function is blocked until the event is called back (once), then it executes the 'actionAtEvent' segment and
/// after unsubscription, exists.
/// </summary>
/// <param name="eventSubscription">The passed delegate must subscribe the event to EventSyncr.EventCallback.</param>
/// <param name="eventCausingAction">The event causing action.</param>
/// <param name="actionAtEvent">The action at event.</param>
/// <param name="eventUnsubscription">Unsubscribe the event that was subscribed in the first parameter</param>
public void Sync(Action eventSubscription, Action eventCausingAction, Action actionAtEvent, Action eventUnsubscription)
{
_actionAtEvent = actionAtEvent;
try
{
eventSubscription();
eventCausingAction();
_eventCallbackEvent.WaitOne();
}
finally
{
eventUnsubscription();
}
}
public void EventCallback(object sender, EventArgs e)
{
try
{
_actionAtEvent();
}
finally
{
_eventCallbackEvent.Set();
}
}
}
While a bit on the verbose side, it does seem to me that it solves all the problems I brought up above:
The code appears just the way it is going to be executed, in its logical order and all the additional properties are self contained.
I am curious is to what you think about the issue and the possible solutions. Which seems the best one? Is there a better way of solving the issue?
I'm aware that in C# 5, the await function will provide a much better solution, but we're not quite there yet :(.
Thanks!
Upvotes: 2
Views: 255
Reputation: 3697
your EventSyncr class is so strange, that you should receive an extra award for this, just Joking ;)
But for real, you only have to create a AutoResetEvent and wait for it at the end of your code using WaitHandle.WaitAll.
At the end of your async method you have to signal the AutoResetEvent and your main thread knows, that your async process has just finished.
public void DoWork()
{
var workId = new Random().Next(100);
Console.WriteLine("Starting a long process ID: " + workId);
LongProcess longProcess = new LongProcess();
longProcess.StartWork();
longProcess.OnWorkMiddle += ()=>{ Console.WriteLine("In middle of long process: " + workId);
longProcess.OnWorkEnd += ()=>{
Console.WriteLine("At the end of a long process");
autoEvent.Set();
};
WaitHandle.WaitAll(new []{longProcess.autoEvent});
}
Upvotes: 1