Xjyuk

Recently, I needed a class that could execute code in a FIFO fashion in order to update parts of a WinForms or WPF UI which did not block the UI Thread and left the UI responsive to any interaction the user did on it.

To achieve that objective, I developed the following code, which meets its requirements and performs decently enough to not be a burden.

using System;
using System.Collections.Concurrent;
using System.ComponentModel;
using System.Threading;

namespace Rayffer.PersonalPortfolio.QueueManagers

 public class BackgroundWorkerActionQueueManager : IDisposable
 
 public bool IsBusy get; private set; 

 private BlockingCollection<Action> actionQueue;
 private readonly BackgroundWorker backgroundWorker;
 private CancellationTokenSource cancellationTokenSource;

 public BackgroundWorkerActionQueueManager()
 
 backgroundWorker = new BackgroundWorker();
 actionQueue = new BlockingCollection<Action>();
 cancellationTokenSource = new CancellationTokenSource();
 backgroundWorker.WorkerSupportsCancellation = true;
 backgroundWorker.DoWork += BackgroundWorker_ManageQueue;
 backgroundWorker.RunWorkerAsync();
 

 public void EnqueueAction(Action actionToEnqueue)
 
 actionQueue.Add(actionToEnqueue);
 

 private void BackgroundWorker_ManageQueue(object sender, DoWorkEventArgs e)
 
 while (!backgroundWorker.CancellationPending)
 
 Action actionToPerform = null;
 try
 
 IsBusy = false;
 actionToPerform = actionQueue.Take(cancellationTokenSource.Token);
 IsBusy = true;
 
 catch (OperationCanceledException ex)
 
 // This exception control intends to capture only when the cancellationToken has been requested to cancel
 break;
 
 if (actionToPerform != null)
 
 actionToPerform.Invoke();
 actionToPerform = null;
 
 
 

 private bool disposedValue = false; // To detect redundant calls

 protected virtual void Dispose(bool disposing)
 
 if (!disposedValue)
 
 if (disposing)
 
 backgroundWorker.CancelAsync();
 cancellationTokenSource.Cancel();
 backgroundWorker.DoWork -= BackgroundWorker_ManageQueue;
 cancellationTokenSource.Dispose();
 backgroundWorker.Dispose();
 

 disposedValue = true;
 
 

 public void Dispose()
 
 Dispose(true);
 
 

This code can be used to perform any action enqueued, from doing a simple sum, to manipulating members of a class ensuring that the operations are carried out in the order they are enqueued.

I was wondering which optimisations I can apply to this code or if there are any bad practices or missusages of the classes.

An example of use would be the following

public class BackGroundWorkerActionQueueManagerTest

 private BackgroundWorkerActionQueueManager sorterActionQueueManager = new BackgroundWorkerActionQueueManager ();
 private BackgroundWorkerActionQueueManager sorterVisualizerActionQueueManager = new BackgroundWorkerActionQueueManager ();
 private bool sorterHasEnded = false;
 public Sorter sorter get; 


 public void StartSorterQueueTest(int[] arrayToSort)
 
 sorterActionQueueManager.EnqueueAction(() =>
 
 sorter.SortAscending(arrayToSort);
 sorterHasEnded = true;
 );
 

 public void StartSorterQueueVisualisation()
 
 sorterVisualizerActionQueueManager.EnqueueAction(() =>
 
 while(!sorterHasEnded)
 
 Thread.Sleep(100);
 
 // Perform operations to visualise the sorting here
 );
 

```

EnqueueAction may want to throw an ObjectDisposedException if the queue is disposed, depending on the precise API you want.

I don't see the value of IsBusy, and it won't be cleared if the task is cancelled.

I little padding inside the while loop would make that code much easier to understand. I'd be temped to pull the logic for retrieving the next action out into its own method so that it can read more clearly:

if (TryGetNextAction(out Action nextAction))

 // invoke etc.

else

 break;

This will make it easier to extend and modify the conditions under which the worker should terminate. You could make it a local function if you want to prevent misuse.

For example, cancellationTokenSource.Token could throw an ObjectDisposedException, which should probably be a case where you fail cleanly, but handling this logic would clutter the existing method.

You never change any of your private members, so you might consider making them readonly. Alternatively you might consider setting them to null upon disposal. Implementation of IDispose looks standard, and I think disposing the BackgroundWorker there is OK, but I'm less than certain.

Some inline documentation would help to confirm the precise API this class is providing, and should clarify things like "if the object is disposed, then pending tasks are dropped", and "order of operations is FIFO as observed from a single thread", etc. BackGroundWorkerActionQueueManagerTest doesn't really elucidate its behaviour.

I think, I would provide the CancellationToken to each action, in order to let them respond to a possible cancellation request. Especially if the action is long running it would be a good idea.

BlockingCollection<Action<CancellationToken>> actionQueue

 while(!sorterHasEnded)
 
 Thread.Sleep(100);
 

This tells me, that you are in the need for a mechanism that tells you when each queued action has finished. May be an event. You could then bind each action to a user defined Id of some kind and signal that, when the action is completed:

private BlockingCollection<ActionItem> actionQueue;


public void EnqueueAction(string id, Action<CancellationToken> actionToEnqueue)

 if (string.IsNullOrWhiteSpace(id)) new ArgumentException("Can not be empty or null", nameof(id));
 if (actionToEnqueue == null) new ArgumentNullException(nameof(actionToEnqueue));

 actionQueue.Add(new ActionItem(id, actionToEnqueue));

And the background worker could be as something like:

private void BackgroundWorker_ManageQueue(object sender, DoWorkEventArgs e)

 while (!backgroundWorker.CancellationPending)
 
 ActionItem actionItem;
 try
 
 IsBusy = false;
 actionItem = actionQueue.Take(cancellationTokenSource.Token);
 IsBusy = true;
 
 catch (OperationCanceledException)
 
 // This exception control intends to capture only when the cancellationToken has been requested to cancel
 break;
 

 actionItem.Action(cancellationTokenSource.Token);
 OnActionEnded(actionItem.Id);
 


public event EventHandler<ActionEndedEventArgs> ActionEnded;

private void OnActionEnded(string id)

 ActionEnded?.Invoke(this, new ActionEndedEventArgs(id));

Where ActionItem is defined as:

class ActionItem

 public ActionItem(string id, Action<CancellationToken> action)
 
 Id = id;
 Action = action;
 

 public string Id get; 
 public Action<CancellationToken> Action get; 

Maybe the ActionEnded event should be fired in a separate thread in order to not block the execution queue while a client responds to it?