Template methode in threaded contexts

Question

Let's say we have a template method that looks like this

abstract class Worker
{
  public void DoJob()
  {
    BeforJob()
    DoRealJob();
    AfterJob();
  }

  abstract void DoRealJob();
}

subclasses that inherit from the Wroker classe should implemente the DoRealJob() method, when the implementation is running under the same thread everything is fine, the three part of the DoJob() method get executed in this order

1. BeforJob()
2. DoRealJob()
3. AfterJob()

but when DoRealJob() runs under another thread, AfterJob() may get executed before DoRealJob() is completed

my actual solution is to let the subclasses call AfterJob() but this doesn't prevent a subclass from forgetting to call it, and we loose the benefit of a template method.

are there other ways to get consistent call order despite the fact the DoRealJob() is blocking or not?

Answer 1

You can't get both the simple inheritance(signature and hooking) and support asynchronous operations in your code.

These two goals are mutually exclusive.

The inheritors must be aware about callback mechanisms in either direct (Tasks, async) or indirect (events, callback functions, Auto(Manual)ResetEvents or other synchronization constructs). Some of them new, some old. And it is difficult to say which one will be better for the concrete case of use.

Well, it may look like there is a simple way with multithreaded code, but what if your DoRealJob will actually run in another process or use some remote job queuing, so the real job will be executed even outside your app?

So:

• If you really consider that your class will be used as the basis for some async worker, then you should design it accordingly.
• If not - do not overengineer. You can't consider any possible scenario. Just document your class well enough and I doubt that anyone will try to implement the DoRealJob asynchronously, especially if you name it DoRealJobSynchronously. If someone tries to do it then in that case your conscience can be pristinely clean.

EDIT:

Do you think it would be correct if I provide both versions, sync and async, of DoRealJob and a flag IsAsynchronous so I can decide which one to call

As I have already said I don't know your actual usage scenarios. And it is unrealistic to consider that the design will be able to effectively handle all of them.

Also there are two very important questions to consider that pertain to your overall Worker class and its DoJob method:

1) You have to determine whether you want the DoJob method to be synchronous or asynchronous, or do you want to have both the synchronous and asynchronous versions? It is not directly related to your question, but it is still very important design decision, because it will have great impact on your object model. This question could be rephrased as:

Do you want the DoJob method to block any actions after it is called until it does its job or do you want to call it as some StartJob method, that will just launch the real processing but it is up to other mechanisms to notify you when the job has ended(or to stop it manually):

        //----------------Sync worker--------------------------
        SyncWorker syncWorker = CreateSyncStringWriter("The job is done");
        Console.WriteLine("SyncWorker will be called now");

        syncWorker.DoJob(); // "The job is done" is written here

        Console.WriteLine("SyncWorker call ended");


        //----------------Async worker--------------------------
        Int32 delay = 1000;
        AsyncWorker asyncWorker = CreateAsyncStringWriter("The job is done", delay);
        Console.WriteLine("AsyncWorker will be called now");

        asyncWorker.StartDoJob(); // "The job is done" won't probably be written here

        Console.WriteLine("AsyncWorker call ended");
        // "The job is done" could be written somewhere here.

2) If you want DoJob to be async(or to have async version) you should consider whether you want to have some mechanisms that will notify when DoJob finishes the processing - Async Programming Patterns , or it is absolutely irrelevant for you when or whether at all it ends.

SO:

Do you have the answers to these two questions?

• If yes - that is good.
• If not - refine and consider your requirements.
• If you are still unsure - stick with simple sync methods.

If you, however, think that you need some async based infrastructure, then, taking into account that it is C# 3.0, you should use Asynchronouse Programming Model.

Why this one and not the event based? Because IAsyncResult interface despite its cumbersomeness is quite generic and can be easily used in Task-based model, simplifying future transition to higher .NET versions.

It will be something like:

 /// <summary>
 /// Interface for both  the sync and async job.
 /// </summary>
 public interface IWorker
{
    void DoJob();

    IAsyncResult BeginDoJob(AsyncCallback callback);

    public void EndDoJob(IAsyncResult asyncResult);
}

/// <summary>
/// Base class that provides DoBefore and DoAfter methods
/// </summary>
public abstract class Worker : IWorker
{
    protected abstract void DoBefore();
    protected abstract void DoAfter();


    public IAsyncResult BeginDoJob(AsyncCallback callback)
    {
        return new Action(((IWorker)this).DoJob)
            .BeginInvoke(callback, null);
    }
    //...
}

public abstract class SyncWorker : Worker
{
    abstract protected void DoRealJobSync();

    public void DoJob()
    {
        DoBefore();
        DoRealJobSync();
        DoAfter();
    }
}

public abstract class AsyncWorker : Worker
{
    abstract protected IAsyncResult BeginDoRealJob(AsyncCallback callback);
    abstract protected void EndDoRealJob(IAsyncResult asyncResult);

    public void DoJob()
    {
        DoBefore();
        IAsyncResult asyncResult = this.BeginDoRealJob(null);
        this.EndDoRealJob(asyncResult);
        DoAfter();
    }
}

P.S.: This example is incomplete and not tested.
P.P.S: You may also consider to use delegates in place of abstract(virtual) methods to express your jobs:

public class ActionWorker : Worker
{
    private Action doRealJob;
    //...

    public ActionWorker(Action doRealJob)
    {
        if (doRealJob == null)
            throw new ArgumentNullException();

        this.doRealJob = doRealJob;
    }

    public void DoJob()
    {
        this.DoBefore();
        this.doRealJob();
        this.DoAfter();
    }
}

DoBefore and DoAfter can be expressed in a similar way.
P.P.P.S: Action delegate is a 3.5 construct, so you will probably have to define your own delegate that accepts zero parameters and returns void.

public delegate void MyAction()

Answer 2

Consider change the DoRealJob to DoRealJobAsync and give it a Task return value. So you can await the eventual asynchronous result.

So your code would look like

abstract class Worker
{
  public void DoJob()
  {
    BeforJob()
    await DoRealJobAsync();
    AfterJob();
  }

  abstract Task DoRealJob();
}

If you don't have .net 4.0 and don't want to us the old 3.0 CTP of async you could use the normale task base style:

  abstract class Worker
    {
      public void DoJob()
      {
        BeforJob()
        var task = DoRealJobAsync();
         .ContinueWith((prevTask) =>
              {
                  AfterJob()
               });
      }

      abstract Task DoRealJob();
    }