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; thereforeleftArray
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
精彩评论