开发者

Factory pattern to instance sorters safe for multi-threading?

First is this a good factory pattern? Update: no - ISorter does not technically fit the correct definition for a factory pattern

Given the following interface:

public interface ISorter
{
    ISorter Initialize(IEnumerable&l开发者_如何转开发t;int> Source);
    void Sort( );

    string Name { get; }
    IEnumerable<int> SortedList { get; }
}

...and 3 sorters - InsertionSort, MergeSort, and QuickSort. Here is an implementation using MergeSort:

[Export(typeof(ISorter))]
public class MergeSorter : ISorter
{
    int[] leftArray;
    int[] rightArray;

    private MergeSorter(IEnumerable<int> source)
    {
        //  independent copies of array
        leftArray = source.ToArray( );
        rightArray = source.ToArray( );
    }

    public ISorter Initialize(IEnumerable<int> Source)
    {
        return new MergeSorter(Source);
    }

    public void Sort( )
    {   /* call merge sort method */   }

    //    assume the rest of ISorter is implemented
    //    as well as the main Sort( ... ) method
}  

The InsertionSort and QuickSort classes follow the identical pattern. Of course, the SortController is completely unaware of what Sorters it is even controlling. It simply provides a list of SorterNames to whatever front end is interested. Now, my question revolves around the implementation of ISorter.Initialize( ). I am returning a newly instanced ISorter object with internal arrays initialized. The intention is that SortController can handle multiple requests to instance multiple sorters; additionally, the Sort method will be executed via the SortController.StartSorter ( ) method:

public static void StartSorter(string SorterName, IEnumerable<int> Source)
{
    //  find ISorter by name and initialize
    //  'sorters' is an internal SorterCollection initialized from MEF discovery
    ISorter sorter = sorters.Sorters
                            .Where(key => key.Name == SorterName)
                            .FirstOrDefault( )
                            .Initialize(Source);

    if (sorter == null)
    { return; }

    SortInfo info = new SortInfo
    {
        Collection = sorter.SortedList,
        Invoker = sorter.Sort, //   delegate for asynchronous execution
        Sorter = sorter,
        Timer = new System.Diagnostics.Stopwatch( )
    };

    info.Timer.Start( );
    info.Invoker.BeginInvoke(new AsyncCallback(SortCompleted), info);

    RunningSorters.Add(sorter);
}

Then in the callback you will see my one and only event defined as follows:

public static event EventHandler<SortCompletedEventArgs> OnSortCompleted = delegate { };

static void SortCompleted ( IAsyncResult iaResult )
{
    //  retrieve SortInfo object from IAsyncResult
    var info = (SortInfo)iaResult.AsyncState;

    //  delegate - EndInvoke
    info.Invoker.EndInvoke ( iaResult );

    //  raise event
    OnSortCompleted ( info, new SortCompletedEventArgs ( info ) );

    //  remove from running sorters list
    RunningSorters.Remove(info.Sorter);
}

Is this a good factory pattern for MEF? And is this a thread-safe and stable implementation capable of handling multiple sorters (say, up to 100,000 integers per sorter)?

Edit

Note: MergeSorter requires the original array to be split into 2 arrays; therefore leftArray and rightArray. Once the array is assigned then we are done with Source and it should be free to change.

The 3 sorting algorithms I used were demonstrative libraries - not my own. My own sorting routine would use the Compare method.

Since this is an extensible SortController employing MEF, all of the sort algorithms have public, parameterless constructors to satisfy instantiation requirement.


IMO, Factory assumes you are to return an instance of a type. I am not too sure this is what the code actually does. I might be wrong, so please correct my logic.

1) Factory interface shall be simple, and only responsible for creation, the name of ISorter doesn't suggest it is a factory

2) ISorter Initialize(IEnumerable<int> Source); I would argue that a method that returns an instance of the current interface is not the best practice, you face some kind of infinite recursion here

3) IEnumerable<int> SortedList { get; } the name suggests it is a sorted list, which is actually a type name in .NET. Such code may confuse other developers

4) MergeSorter > constructor contains two copies of the array - you need twice as much memory for one array, which doubles the memory consumption (is this actually required for this sort algorithm?)

5) It is not thread safe, as you are using ToArray() for the same array (into two private fields). There is no synchronization there, so the array may change between these two calls, leading to nasty exceptions

To sum up

I would suggest to make:

1) A thin and dedicated factory interface

2) Move sorting functionality to implementing the IComparer

3) This requires quite a lot of refactoring, so if this code works, just add the locks where you copy/delete/modify the arrays (to make it thread safe) and remove (if possible) the duplicated copies

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜