Is there a better waiting pattern for c#?
A much better way to implement this pattern is to have your Thing
object expose an event on which the consumer can wait. For example a ManualResetEvent
or AutoResetEvent
. This greatly simplifies your consumer code to be the following
if (!Thing.ManualResetEvent.WaitOne(sleep_time)) {
throw new ItDidntHappen();
}
// It happened
The code on the Thing
side is also not really any more complex.
public sealed class Thing {
public readonly ManualResetEvent ManualResetEvent = new ManualResetEvent(false);
private void TheAction() {
...
// Done. Signal the listeners
ManualResetEvent.Set();
}
}
Use events.
Have the thing you are waiting for raise an event when it's finished (or failed to finish within the allotted time) and then handle the event in your main application.
That way you don't have any Sleep
loops.
A loop is not a TERRIBLE way to wait for something, if there's nothing else for your program to do while it waits (for instance while connecting to a DB). However, I see some issues with yours.
//It's not apparent why you wait exactly 10 times for this thing to happen
for (int i = 0; i < 10; i++)
{
//A method, to me, indicates significant code behind the scenes.
//Could this be a property instead, or maybe a shared reference?
if (Thing.WaitingFor())
{
break;
}
//Sleeping wastes time; the operation could finish halfway through your sleep.
//Unless you need the program to pause for exactly a certain time, consider
//Thread.Yield().
//Also, adjusting the timeout requires considering how many times you'll loop.
Thread.Sleep(sleep_time);
}
if(!Thing.WaitingFor())
{
throw new ItDidntHappenException();
}
In short, the above code looks more like a "retry loop", that's been bastardized to work more like a timeout. Here's how I would structure a timeout loop:
var complete = false;
var startTime = DateTime.Now;
var timeout = new TimeSpan(0,0,30); //a thirty-second timeout.
//We'll loop as many times as we have to; how we exit this loop is dependent only
//on whether it finished within 30 seconds or not.
while(!complete && DateTime.Now < startTime.Add(timeout))
{
//A property indicating status; properties should be simpler in function than methods.
//this one could even be a field.
if(Thing.WereWaitingOnIsComplete)
{
complete = true;
break;
}
//Signals the OS to suspend this thread and run any others that require CPU time.
//the OS controls when we return, which will likely be far sooner than your Sleep().
Thread.Yield();
}
//Reduce dependence on Thing using our local.
if(!complete) throw new TimeoutException();
If possible, have the asynchronous processing wrapped in a Task<T>
. This provides the best of all worlds:
- You can respond to the completion in an event-like way by using task continuations.
- You can wait using the completion's waitable handle because
Task<T>
implementsIAsyncResult
. - Tasks are easily composable using the
Async CTP
; they also play well withRx
. - Tasks have a very clean built-in exception handling system (in particular, they correctly preserve the stack trace).
If you need to use a timeout, then Rx or the Async CTP can provide that.
I would take a look at the WaitHandle class. Specifically the ManualResetEvent class that waits until the object is set. You can also specify timeout values for it and check if it was set afterward.
// Member variable
ManualResetEvent manual = new ManualResetEvent(false); // Not set
// Where you want to wait.
manual.WaitOne(); // Wait for manual.Set() to be called to continue here
if(!manual.WaitOne(0)) // Check if set
{
throw new ItDidntHappenException();
}