Don't Repeat Your Threading Code

Often when writing Windows Forms applications I will need to perform a long-running action on a background thread. So I write some code that can kick a delegate off in another thread (probably using the ThreadPool) and then some more code that handles when the action has completed. Then there is the code that handles any exceptions that were raised during that background action. Then there is the code that marshals back to the GUI thread so I can actually update controls on the form. Then I often end up adding a framework for reporting progress, and for aborting the action.

The end result is that the actual code that implements the task at hand has been obscured by a huge pile of threading related lines of code. And very often this code gets replicated every time a new action that needs to run on a background thread is written.

So I decided to see how far I could go with a DRY approach - ("Don't Repeat Yourself"), and decided to create a base BackgroundAction class that would hide a lot of the heavy lifting.

The first step was to create an interface, IBackgroundAction. The key components are the Begin method, that starts the action, and the Finished event, which indicates that the action has finished (successfully or not). I also put in a basic Progress reporting mechanism, as well as a way to request a Cancel of the action.

public interface IBackgroundAction
{
    void Begin();
    event EventHandler<ActionFinishedEventArgs> Finished;
    event EventHandler<ProgressEventArgs> Progress;
    void Cancel();
    ActionState State { get; }
}

public class ActionFinishedEventArgs : EventArgs
{
    public ActionState State { get; set; }
    public Exception Exception { get; set; }
}

public class ProgressEventArgs : EventArgs
{
    public string Message { get; set; }
}

public enum ActionState
{
    None,
    InProgress,
    Success,
    Cancelled,
    Error
}

The next step was to create an abstract base class, BackgroundAction that implements the interface, and does as much of the work as is possible:

abstract class BackgroundAction : IBackgroundAction
{
    protected volatile bool cancel;
    ActionState state = ActionState.None;
    public event EventHandler<ActionFinishedEventArgs> Finished;
    public event EventHandler<ProgressEventArgs> Progress;

    public void Begin()
    {
        if (state != ActionState.None)
        {
            throw new InvalidOperationException("Begin should only be called once");
        }
        state = ActionState.InProgress;
        ThreadPool.QueueUserWorkItem(new WaitCallback(ThreadProc));
    }

    void ThreadProc(object state)
    {
        try
        {
            PerformBackgroundAction();
            state = cancel ? ActionState.Cancelled : ActionState.Success;
            ReportFinished();
        }
        catch (Exception e)
        {
            state = ActionState.Error;
            if (Finished != null)
            {
                Finished(this, new ActionFinishedEventArgs() { State = State, Exception = e });
            }
        }
    }

    protected abstract void PerformBackgroundAction();

    public void Cancel()
    {
        cancel = true;
    }

    public ActionState State
    {
        get { return state; }
    }

    void ReportFinished()
    {
        if (Finished != null)
        {
            Finished(this, new ActionFinishedEventArgs() { State = state });
        }
    }

    protected void ReportProgress(string message, params object[] args)
    {
        if (Progress != null)
        {
            Progress(this, new ProgressEventArgs() { Message = String.Format(message, args) });
        }
    }
}

This class manages the launching of the background action and the reporting of its completion (whether successful or not). It also includes a ReportProgress function as a helper for the concrete class to use.

Moving all this threading related code into a base class means that whenever we create a new action, the code we write is much cleaner, and only has the additional concerns of cancel checking and reporting progress. Here's an example from the project I was trying this out on. All I had to do was override the PerformBackgroundAction function to do the task at hand...

class DeserializeAllAction : BackgroundAction
{
    IEnumerable<BinarySearchResult> searchResults;

    public DeserializeAllAction(IEnumerable<BinarySearchResult> 
                                searchResults)
    {
        this.searchResults = searchResults;
    }

    protected override void PerformBackgroundAction()
    {
        ReportProgress("Preparing to deserialize");
        int count = 0;
        foreach (BinarySearchResult result in searchResults)
        {
            if (cancel)
            {
                break;
            }
            if (++count % 10 == 0)
            {
                ReportProgress("Deserialized {0}", count);
            }
            result.Deserialize();
        }
    }
}

The next step was to create a Progress Form that could take any IBackgroundAction, and run it, while showing progress updates and allowing the user to cancel. Here's a simple implementation:

public partial class ProgressForm : Form
{
    IBackgroundAction backgroundAction;
    bool finished;
    
    public ProgressForm(string title, IBackgroundAction 
                        backgroundAction)
    {
        this.backgroundAction = backgroundAction;
        backgroundAction.Finished += new 
            EventHandler<ActionFinishedEventArgs>
               (backgroundAction_Finished);
        backgroundAction.Progress += new 
            EventHandler<ProgressEventArgs>
                (backgroundAction_Progress);
        InitializeComponent();
        this.FormClosing += new 
            FormClosingEventHandler(ProgressForm_FormClosing);
        this.labelProgressMessage.Text = String.Empty;
        this.Text = title;
    }

    void ProgressForm_FormClosing(object sender, 
       FormClosingEventArgs e)
    {
        if (!finished)
        {
            backgroundAction.Cancel();
            buttonCancel.Enabled = false;
            e.Cancel = true;
        }
    }

    void backgroundAction_Progress(object sender, 
       ProgressEventArgs e)
    {
        if (this.InvokeRequired)
        {
            this.BeginInvoke(new 
               EventHandler<ProgressEventArgs>
                  (backgroundAction_Progress), sender, e);
            return;
        }
        labelProgressMessage.Text = e.Message;
    }

    void backgroundAction_Finished(object sender, 
        ActionFinishedEventArgs e)
    {
        if (this.InvokeRequired)
        {
            this.BeginInvoke(new  
               EventHandler<ActionFinishedEventArgs>
                  (backgroundAction_Finished), sender, e);
            return;
        }
        if(e.State == ActionState.Error)
        {
            MessageBox.Show(this, e.Exception.Message,
                this.Title,
                MessageBoxButtons.OK, MessageBoxIcon.Warning);
        }
        finished = true;
        this.Close();
    }

    private void ProgressForm_Load(object sender, EventArgs e)
    {
        backgroundAction.Begin();
    }
}

Now the code to create and run my background action becomes trivial:

private void buttonDeserializeAll_Click(object sender, EventArgs e)
{
    DeserializeAllAction deserializer = new DeserializeAllAction(searchResults);
    ProgressForm progressForm = new ProgressForm("Deserializing all results", deserializer);
    progressForm.ShowDialog(this);
}

So I was quite pleased with what I had achieved, but I started wondering whether I could take things one step further. In our PerformBackgroundAction function, we still have to perform periodic checks for cancellation and progress updates. These can't be moved down into the base class because it doesn't know when it can check and what it can report.

But then I considered the fact that most long-running background operations are in fact working their way through some kind of list. If I could insert the progress reporting and cancel checking into the enumerator, the PerformBackgroundAction function could focus entirely on doing the action at hand.

Here's my ProgressEnumerator class that injects progress reporting and cancel checking into any IEnumerable type. It calls back once every time round the loop, allowing a progress message to be emitted (if required), and allowing cancellation to be signalled (by returning true from the callback):

delegate bool ProgressCallback<T>(T value, int count);

class ProgressEnumerator<T>
{
    public static IEnumerable<T> Create(IEnumerable<T> items, ProgressCallback<T> callback)
    {
        ProgressEnumerator<T> enumerator = new ProgressEnumerator<T>(items, callback);
        return enumerator.Items;
    }

    IEnumerable<T> items;
    ProgressCallback<T> callback;
    int count;

    public ProgressEnumerator(IEnumerable<T> items, ProgressCallback<T> callback)
    {
        this.items = items;
        this.callback = callback;
        count = 0;
    }

    public IEnumerable<T> Items
    {
        get
        {
            foreach (T item in items)
            {
                if(callback(item,++count))
                    break;
                yield return item;
            }
        }
    }
}

Now we can revisit our PerformBackgroundAction function and remove the progress reporting and cancellation checking from inside the foreach loop, and moving it out into a separate function.

protected override void PerformBackgroundAction()
{
    ReportProgress("Preparing to deserialize");
    foreach (BinarySearchResult result in 
        ProgressEnumerator<BinarySearchResult>.Create(searchResults,ProgressCallback))
    {
        result.Deserialize();
    }
}

bool ProgressCallback(BinarySearchResult result, int count)
{
    if (count % 10 == 0)
    {
        ReportProgress("Deserialized {0}", count);
    }
    return cancel;
}

Finally I have achieved what I wanted. The GUI code that launches the background action is simple, and the code that performs the background action - the real 'business logic' - is unencumbered with threading related noise.

I'm sure that both my BackgroundAction and ProgressEnumerator classes have lots of ways in which they can be improved or streamlined further. Feel free to critique them in the comments.