views:

133

answers:

1

How could I modify the below code such that I could, for better readability in code:

a) move the "workThreadMethod()" into it's own class

b) not have any code in this worker thread class reference static variables from the main "Program" class

c) the above are the main two requirements, however I'm hoping that as a side effect this would then ensure that for testability the worker thread class methods would be easier to test and ideally lend itself to testing via the IOC (e.g. Ninject) concept [if this doesn't make sense then ignore this point for the purpose of the question]

The main challenge that I'm not sure about re solving is how to handle the two different shared variables between the originating thread & new thread (one of them a ConcurrentQueue which the new thread adds to, and the other a bool variable that the original thread uses to indicate to the new thread when to stop)

using System.Collections.Concurrent;
using System.Diagnostics;
using System.Threading;

namespace TestConsoleApp
{
    class Program
    {
        // Main Thread uses to indicate to New Thread to stop
        private static bool _shouldStop = false;

        // New Thread uses to pass back result to Main Thread
        private static long _results = 0;

        // Main Thread passes ongoing updates to New Thread via this queue
        private static ConcurrentQueue<long> _workQueue = new ConcurrentQueue<long>();

        static void Main(string[] args)
        {
            var p = new Program();
            p.TestThreads();
        }

        public void TestThreads()
        {
            _shouldStop = false;
            var workThread = new Thread(workThreadMethod);
            workThread.Start();

            for (int i = 0; i < 100; i++)
            {
                _workQueue.Enqueue(i);   // Add test data to queue
                Debug.WriteLine("Queue  : " + i);
                Thread.Sleep(10);
            }

            Thread.Sleep(5000);

            _shouldStop = true;
            workThread.Join();
            Debug.WriteLine("Finished TestThreads.  Result = " + _results);
        }


        // Dequeuer Methods
        private void workThreadMethod()
        {
            // Update Summary
            while (!_shouldStop)
            {
                if (_workQueue.Count == 0)
                {
                    Thread.Sleep(10);
                }
                else
                {
                    long currentValue;
                    bool worked = _workQueue.TryDequeue(out currentValue);
                    if (worked)
                    {
                        _results += currentValue;
                        Debug.WriteLine("DeQueue: " + currentValue);
                    }
                }
            }
        }
    }
}
+2  A: 

This is an exercise in separation of concerns, and initially I would split this program into a work provider and worker. The provider is responsible for the queue and execution control while the worker should do calculation. The following code is a crude start but it should get you going.

Splitting up the two concerns and using constructor injection already pays of in testability, you can now fully test Worker without involving Program.

Note: considering further development of your app I would strongly suggest that you look into the Task Parallel Library. Using a library such as the TPL enables you to take advantage of multi-core processors without having to deal with the complexities of thread allocation and work scheduling. More resources on the TPL is discussed here.

public interface IWorkProvider
{
    bool ShouldStop { get; }
    long? GetWork();
}

public class Program : IWorkProvider
{
    // Main Thread uses to indicate to New Thread to stop
    private static bool _shouldStop = false;

    // Main Thread passes ongoing updates to New Thread via this queue
    private static ConcurrentQueue<long> _workQueue = new ConcurrentQueue<long>();

    public bool ShouldStop { get { return _shouldStop; } }

    public long? GetWork()
    {
        long currentValue;
        bool worked = _workQueue.TryDequeue(out currentValue);
        if (worked)
            return currentValue;
        return null;
    }
}

public class Worker
{
    private long _results;
    private readonly IWorkProvider _workProvider;

    public long Results { get { return _results; }}

    public Worker(IWorkProvider workProvider)
    {
        _workProvider = workProvider;
    }

    public void DoWork()
    {
        // Update Summary
        while (!_workProvider.ShouldStop)
        {
            long? work = _workProvider.GetWork();
            if (work.HasValue)
            {
                _results += work.Value;
                Debug.WriteLine("DeQueue: " + work.Value);
            }
            else
            {
                Thread.Sleep(10);
            }
        }
    }

}
Peter Lillevold
@Ruben - thanks, added the link to my answer.
Peter Lillevold
@Peter - thanks for this. My next step will be to try to translate this concept to some "concurrent" work I have going on using Quartz.Net which already has a "IJob" interface if I can...
Greg