snippetcsharpMinor
Trying to create a robust task execution engine
Viewed 0 times
enginerobustcreatetryingexecutiontask
Problem
I've created a simple asynchronous execution engine and I would like some help identifying places it can be improved. The basic function of the system is to monitor MSMQ and Service Broker queues and execute any tasks that come through on them.
Here is the code:
```
using System;
using System.Collections.Generic;
using System.Linq;
using Castle.Windsor;
using ThreadingTasks = System.Threading.Tasks;
public class Engine
{
///
/// Start's the Engine and spins up appropriate message listeners
///
public void Start()
{
_logger.Info("Starting the Engine, calling GetAsync for all queues");
_cancelToken = false;
GetAsync(AsyncLaunch.All);
}
///
/// Sends the stop message to the engine which will let currently running tasks finish
/// and stop future tasks from being picked up.
///
public void Stop()
{
_logger.Info("Stopping the Engine");
_cancelToken = true;
}
protected enum AsyncLaunch { All, MSMQ, ServiceBroker };
///
/// Starts all appropriate asynchronous Message listeners
///
///
protected void GetAsync(AsyncLaunch async)
{
if (_cancelToken)
return;
_logger.Debug("Attempting to get messages to process Asynchronously");
if (SingleThreaded)
{
_logger.Debug("Engine is running in single-threaded mode and must wait till all tasks finish");
ThreadingTasks.Task.WaitAll(_threadTasks.ToArray());
_threadTasks.Clear();
}
if (async == AsyncLaunch.ServiceBroker || async == AsyncLaunch.All)
_serviceBroker.GetAsync(x => RunServiceBrokerJob(x));
if (async == AsyncLaunch.MSMQ || async == AsyncLaunch.All)
_msmq.GetManyAsync(x => RunMSMQJob(x));
}
///
/// Runs the job for the given message and after the job has launched, calls
/// GetAsync to get the next ServiceBroker message
///
protected void Ru
Here is the code:
```
using System;
using System.Collections.Generic;
using System.Linq;
using Castle.Windsor;
using ThreadingTasks = System.Threading.Tasks;
public class Engine
{
///
/// Start's the Engine and spins up appropriate message listeners
///
public void Start()
{
_logger.Info("Starting the Engine, calling GetAsync for all queues");
_cancelToken = false;
GetAsync(AsyncLaunch.All);
}
///
/// Sends the stop message to the engine which will let currently running tasks finish
/// and stop future tasks from being picked up.
///
public void Stop()
{
_logger.Info("Stopping the Engine");
_cancelToken = true;
}
protected enum AsyncLaunch { All, MSMQ, ServiceBroker };
///
/// Starts all appropriate asynchronous Message listeners
///
///
protected void GetAsync(AsyncLaunch async)
{
if (_cancelToken)
return;
_logger.Debug("Attempting to get messages to process Asynchronously");
if (SingleThreaded)
{
_logger.Debug("Engine is running in single-threaded mode and must wait till all tasks finish");
ThreadingTasks.Task.WaitAll(_threadTasks.ToArray());
_threadTasks.Clear();
}
if (async == AsyncLaunch.ServiceBroker || async == AsyncLaunch.All)
_serviceBroker.GetAsync(x => RunServiceBrokerJob(x));
if (async == AsyncLaunch.MSMQ || async == AsyncLaunch.All)
_msmq.GetManyAsync(x => RunMSMQJob(x));
}
///
/// Runs the job for the given message and after the job has launched, calls
/// GetAsync to get the next ServiceBroker message
///
protected void Ru
Solution
-
-
-
enums can be used as flags:
Then your testing code becomes:
plus you can pass in an arbitrary combination of targets (in case you add more later).
-
-
You have a lot of duplicate code in your two
GetAsync is a bad name for that method. StartListening seems to express better what it does.-
AsyncLaunch is a bad name for that enum. It seems to describe which target should be listened to so why not call it ListenTarget.-
enums can be used as flags:
enum ListenTarget
{
MSMQ = 1 << 0,
ServiceBroker = 1 << 1,
All = ~0,
}Then your testing code becomes:
if (async & AsyncLaunch.ServiceBroker)
_serviceBroker.GetAsync(x => RunServiceBrokerJob(x));
if (async & AsyncLaunch.MSMQ)
_msmq.GetManyAsync(x => RunMSMQJob(x));plus you can pass in an arbitrary combination of targets (in case you add more later).
-
ValidateIsNotNull should be renamed to state more clearly what it does: ThrowIfObjectIsNull.-
You have a lot of duplicate code in your two
RunJob methods. The single message one could be shortened to one line of code RunJobs(new [] { msg }).Code Snippets
enum ListenTarget
{
MSMQ = 1 << 0,
ServiceBroker = 1 << 1,
All = ~0,
}if (async & AsyncLaunch.ServiceBroker)
_serviceBroker.GetAsync(x => RunServiceBrokerJob(x));
if (async & AsyncLaunch.MSMQ)
_msmq.GetManyAsync(x => RunMSMQJob(x));Context
StackExchange Code Review Q#5204, answer score: 3
Revisions (0)
No revisions yet.