debugcsharpMinor
Tasks: exceptions and cancelation
Viewed 0 times
andexceptionstaskscancelation
Problem
I need to do a long running task. I've done this by executing a Task while there's a loading box on the UI. When an exception is thrown, I want to stop the task and show a msgbox to the user. If everything goes right, I stop the loading box.
The code below works as expected, but I was wondering if I'm doing it correct here since this is the first time I'm doing something like this.
MyClassLibrary
Baseclass:
```
private readonly CancellationTokenSource cancellationTokenSource = new CancellationTokenSource();
public MyEnum Stage {get; protected set;}
public event EventHandler ProgrammingStarted;
public event EventHandler ProgrammingExecuted;
protected void ProgramImage()
{
this.OnProgrammingStarted(new EventArgs());
var task =
Task.Factory.StartNew(this.ProgramImageAsync)
.ContinueWith(
this.TaskExceptionHandler,
cancellationTokenSource.Token,
TaskContinuationOptions.OnlyOnFaulted,
TaskScheduler.FromCurrentSynchronizationContext()) //Catch exceptions here
.ContinueWith(
o => this.ProgramImageAsyncDone(),
cancellationTokenSource.Token,
TaskContinuationOptions.NotOnFaulted,
TaskScheduler.FromCurrentSynchronizationContext()); //Run this when no exception occurred
}
private void ProgramImageAsync()
{
Thread.Sleep(5000); // Actual programming is done here
throw new Exception("test");
}
private void TaskExceptionHandler(Task task)
{
var exception = task.Exception;
if (exception != null && exception.InnerExceptions.Count > 0)
{
this.OnProgrammingExecuted(
new ProgrammingExecutedEventArgs { Success = false, Error = exception.InnerExceptions[0] });
this.Explanation = "An error occurrred duri
The code below works as expected, but I was wondering if I'm doing it correct here since this is the first time I'm doing something like this.
MyClassLibrary
Baseclass:
```
private readonly CancellationTokenSource cancellationTokenSource = new CancellationTokenSource();
public MyEnum Stage {get; protected set;}
public event EventHandler ProgrammingStarted;
public event EventHandler ProgrammingExecuted;
protected void ProgramImage()
{
this.OnProgrammingStarted(new EventArgs());
var task =
Task.Factory.StartNew(this.ProgramImageAsync)
.ContinueWith(
this.TaskExceptionHandler,
cancellationTokenSource.Token,
TaskContinuationOptions.OnlyOnFaulted,
TaskScheduler.FromCurrentSynchronizationContext()) //Catch exceptions here
.ContinueWith(
o => this.ProgramImageAsyncDone(),
cancellationTokenSource.Token,
TaskContinuationOptions.NotOnFaulted,
TaskScheduler.FromCurrentSynchronizationContext()); //Run this when no exception occurred
}
private void ProgramImageAsync()
{
Thread.Sleep(5000); // Actual programming is done here
throw new Exception("test");
}
private void TaskExceptionHandler(Task task)
{
var exception = task.Exception;
if (exception != null && exception.InnerExceptions.Count > 0)
{
this.OnProgrammingExecuted(
new ProgrammingExecutedEventArgs { Success = false, Error = exception.InnerExceptions[0] });
this.Explanation = "An error occurrred duri
Solution
Since you're using Task-based async processing it's better to declare long-running method as returning
Then all you need on UI side is to have an asynchronous method like this:
UPDATE (based on updated question):
Your solution doesn't have proper separation of concerns. The class containing
In order to properly separate concerns you should move all the logic related to UI to your user control, and leave only business logic in the
In order to do that you need to decide what is the actual trigger for showing a loading box. I'll stick to assumption that the actual reason to show a loading box is not a "programming" stage, but a long-running process, i.e. if you would add a new stage that may also take a long time to complete you may also want to show the loading box there.
The next question to answer is how do you want to detect a long-running stage? I can suggest 2 options:
I'll use the first option in my example as it requires less changes to my previous code:
Business logic:
UI:
Task or Task object:public async Task ProgramImageAsync()
{
await Task.Delay(TimeSpan.FromSeconds(5)); // Actual programming is done here
throw new DivideByZeroException("test");
}Then all you need on UI side is to have an asynchronous method like this:
private async void RunLongUIPreparation()
{
//TODO: Show loading box here
var processor = new YourClassContainingProgramImageAsync();
try
{
await processor.ProgramImageAsync();
//TODO: Hide loading box here
}
catch (DivideByZeroException exception)
{
//TODO: Show messagebox with warning here
}
}UPDATE (based on updated question):
Your solution doesn't have proper separation of concerns. The class containing
ProgramImage method (MyDerivedClass) knows about UI as it handles strings shown on UI (e.g. Explanation property), events (even though they are named as a business flow events) are designed specifically for UI interaction.In order to properly separate concerns you should move all the logic related to UI to your user control, and leave only business logic in the
MyDerivedClass. In order to do that you need to decide what is the actual trigger for showing a loading box. I'll stick to assumption that the actual reason to show a loading box is not a "programming" stage, but a long-running process, i.e. if you would add a new stage that may also take a long time to complete you may also want to show the loading box there.
The next question to answer is how do you want to detect a long-running stage? I can suggest 2 options:
- show a loading box if stage hasn't completed within a certain timeout (set a timeout inside UI control);
- add a progress notification support to
MyDerivedClassso that it can notify clients about its progress (using anIProgressinterface specifically designed for asynchronous progress notification).
I'll use the first option in my example as it requires less changes to my previous code:
Business logic:
public async Task ProgramImageAsync()
{
await Task.Delay(TimeSpan.FromSeconds(5)); // Actual programming is done here
throw new DivideByZeroException("test");
}UI:
private async void RunLongUIPreparation()
{
var processor = new YourClassContainingProgramImageAsync();
try
{
Task stageTask = processor.ProgramImageAsync();
ShowLoadingBoxIfRunningTooLong(stageTask);
await processor.ProgramImageAsync();
}
catch (DivideByZeroException exception)
{
//TODO: Show messagebox with warning here
}
}
private async void ShowLoadingBoxIfRunningTooLong(Task stageTask)
{
await Task.Delay(TimeSpan.FromMilliseconds(100));
if (stageTask.IsCompleted)
return;
try
{
this.ShowLoadingBox();
await stageTask;
}
finally
{
this.RemoveLoadingBox();
}
}Code Snippets
public async Task ProgramImageAsync()
{
await Task.Delay(TimeSpan.FromSeconds(5)); // Actual programming is done here
throw new DivideByZeroException("test");
}private async void RunLongUIPreparation()
{
//TODO: Show loading box here
var processor = new YourClassContainingProgramImageAsync();
try
{
await processor.ProgramImageAsync();
//TODO: Hide loading box here
}
catch (DivideByZeroException exception)
{
//TODO: Show messagebox with warning here
}
}public async Task ProgramImageAsync()
{
await Task.Delay(TimeSpan.FromSeconds(5)); // Actual programming is done here
throw new DivideByZeroException("test");
}private async void RunLongUIPreparation()
{
var processor = new YourClassContainingProgramImageAsync();
try
{
Task stageTask = processor.ProgramImageAsync();
ShowLoadingBoxIfRunningTooLong(stageTask);
await processor.ProgramImageAsync();
}
catch (DivideByZeroException exception)
{
//TODO: Show messagebox with warning here
}
}
private async void ShowLoadingBoxIfRunningTooLong(Task stageTask)
{
await Task.Delay(TimeSpan.FromMilliseconds(100));
if (stageTask.IsCompleted)
return;
try
{
this.ShowLoadingBox();
await stageTask;
}
finally
{
this.RemoveLoadingBox();
}
}Context
StackExchange Code Review Q#27021, answer score: 2
Revisions (0)
No revisions yet.