Strategy for choosing the right object and the right method
in the code below, first the if
statement block (there will be more than just the worker condition associated with else if
) and select the correct one filter_object
. After that, in the same conditional block, I choose which filter should be applied by the filter object. This code is silly.
public class Filter
{
public static List<data.Issue> fetch(string type, string filter)
{
Filter_Base filter_object = new Filter_Base(filter);
if (type == "worker")
{
filter_object = new Filter_Worker(filter);
}
else if (type == "dispatcher")
{
filter_object = new Filter_Dispatcher(filter);
}
List<data.Issue> result = new List<data.Issue>();
if (filter == "new")
{
result = filter_object.new_issues();
}
else if (filter == "ended")
{
result = filter_object.ended_issues();
}
return result;
}
}
public class Filter_Base
{
protected string _filter;
public Filter_Base(string filter)
{
_filter = filter;
}
public virtual List<data.Issue> new_issues()
{
return new List<data.Issue>();
}
public virtual List<data.Issue> ended_issues()
{
return new List<data.Issue>();
}
}
public class Filter_Worker : Filter_Base
{
public Filter_Worker(string filter) :
base(filter)
{ }
public override List<data.Issue> new_issues()
{
return (from i in data.db.GetInstance().Issues
where (new int[] { 4, 5 }).Contains(i.RequestStatusId)
select i).Take(10).ToList();
}
}
public class Filter_Dispatcher : Filter_Base
{
public Filter_Dispatcher(string filter) :
base(filter)
{ }
}
it will be used in some form:
Filter.fetch("worker", "new");
this code means that for a user belonging to the "worker" role, only "new" problems will be selected (this is some kind of small and simple CRM). Or another:
Filter.fetch("dispatcher", "ended"); // here we get finished issues for dispatcher role
Any suggestions on how to improve it?
a source to share
DI is a possibility if there are loads of FilterBase derivatives. Depending on your weapon selection code, it looks like this:
ioc
.RegisterType<IDataStrategy, FilterWorker>("worker")
/// more filters
.RegisterType<IDataStrategy, FilterDispatcher>("dispatcher");
Alternatively, the factory object can at least refactor your code, but I digress ...
while you're at it, you lose the base class and implement the interface:
public interface IDataStrategy
{
IEnumerable<Data.Issue> FetchNew();
IEnumerable<Data.Issue> FetchEnded();
}
Use an enum as suggested:
public enum FilterType
{
New,
Ended
}
Now in your Fetch function
public IEnumerable<Data.Issue> Fetch(string dataType, FilterType filterType)
{
var strategy = ioc.Resolve<IDataStrategy>(dataType);
IEnumerable<Data.Issue> results = null;
switch(filterType)
{
case FilterType.New:
results = strategy.FetchNew();
default:
results = strategy.Ended();
}
return results;
}
a source to share
I assume you are asking how you can trim the Fetch method. I would use generics
public static List<data.Issue> Fetch<T>( string filter ) where T : FilterBase, new()
{
var filterBase = new T();
filterBase.Initialize( filter );
List<data.Issue> result;
if ( IsNew( filter ) )
result = filterBase.NewIssues();
else if ( IsEnded( filter ) )
result = filterBase.EndedIssues();
else
result = new List<data.Issue>();
return result;
}
This requires:
- Unsigned public (or at least internal) constructor for FilterBase and derived classes.
- A named virtual method
Initialize
that takes a string in FilterBase. - Static function IsNew and IsEnded in your class
Filter
so that the methodFetch
can determine which function you can use. Another solution would be to have an additional delegate parameterFetch
to determine which method should be called.
How can you improve the rest of the code?
- Stop using underscores in class and method names (and field names).
- Use Pascal Case for method names (so NewIssues instead of new_issues)
- Make your base class abstract unless there is no good reason otherwise.
- Find a more definitive means of defining "new" and "finished." Possibly an optional enum parameter?
a source to share
This may sound long, but I would use attributes and reflections to try and achieve this.
Sort of
Attributes:
class FilterType : Attribute
{
public string Filter;
public FilterType(string filter)
{
Filter = filter;
}
}
class FilterMethod : Attribute
{
public string Filter;
public FilterMethod(string filter)
{
Filter = filter;
}
}
Classes with attribute markers
public class Filter_Base
{
protected string _filter;
public Filter_Base(string filter)
{
_filter = filter;
}
[FilterMethod("new")]
public virtual List<string> new_issues()
{
return new List<string>();
}
}
[FilterType("Worker")]
public class Filter_Worker : Filter_Base
{
public Filter_Worker(string filter) :
base(filter)
{ }
public override List<string> new_issues()
{
return new List<string>();
}
}
Filter method
private static List<string> GetInstanceList(string type, string filter)
{
//get all classes implementing FilterType Attribute
var dict = AppDomain.CurrentDomain.GetAssemblies().
SelectMany(x => x.GetTypes()).
Where(x => x.GetCustomAttributes(typeof(FilterType), false).Length > 0).
Select(x => new { ((FilterType)x.GetCustomAttributes(typeof(FilterType), false)[0]).Filter, x }).
ToDictionary(x => x.Filter);
Filter_Base instance = (Filter_Base)Activator.CreateInstance(dict[type].x, filter);
var methods = instance.GetType().GetMembers().
Where(x => x.GetCustomAttributes(typeof(FilterMethod), true).Length > 0).
Select(x => new { ((FilterMethod)x.GetCustomAttributes(typeof(FilterMethod), true)[0]).Filter, x }).
ToDictionary(x => x.Filter);
return (List<string>)instance.GetType().GetMethod(methods[filter].x.Name).Invoke(instance, null);
}
Finally, call
List<string> instance = GetInstanceList("Worker", "new");
a source to share