Background
This is a refactoring question. I have a bunch of methods that more or less have exactly the same code but they act on different types. There is essentially one method per type and I want to combine them all into one that can use a generic type.
Current Code
Perhaps the below code will help explain what I'm trying -
The below methods differ mostly in the DbSet<> entity argument. Inside the method code, they use mostly exactly the same properties but in one or two lines they may use properties that are not shared by the entity types. For example, AccountId (from Account entity) and CustomerId (from Customer entity).
int? MethodToRefactor(DbSet<Account> entity, List someCollection, string[] moreParams)
{
int? keyValue = null;
foreach (var itemDetail in someCollection)
{
string refText = GetRefTextBySource(itemDetail, moreParams);
//Only the below two lines differ in all MethodToRefactor because they use entity's properties that are not shared by all entities
if (entity.Count(a => a.Name == refText) > 0)
keyValue = entity.Where(a => a.Name == refText).First().AccountId;
if (...some conditional code...)
break;
}
return keyValue;
}
int? MethodToRefactor(DbSet<Customer> entity, List someCollection, string[] moreParams)
{
int? keyValue = null;
foreach (var itemDetail in someCollection)
{
string refText = GetRefTextBySource(itemDetail, moreParams);
//Only the below two lines differ in all MethodToRefactor because they use entity's properties that are not shared by all entities
if (entity.Count(c => c.CustomerName == refText) > 0)
keyValue = entity.Where(c => c.CustomerName == refText).First().CustomerId;
if (...some conditional code...)
break;
}
return keyValue;
}
Below is the code that calls the above methods -
void Caller()
{
foreach (var entity in EntityCollection)
{
if (entity.Name == "Account")
{
id = MethodToRefactor(db.Accounts,...);
}
else if (entity.Name == "Customer")
{
id = MethodToRefactor(db.Customers,...);
}
}
}
Problem
This is not scalable for one thing because it requires copying/pasting a new MethodToRefactor for each newly added entity. It is difficult to maintain as well. I can perhaps refactor the code common to all MethodToRefactors in a separate method and do an ifelse inside it per entity but then I would basically be merging the Caller with MethodToRefactor. I'm looking for a neater solution with minimal changes in Caller method, as described below.
Ideal/desired refactored code
This is a great candidate for generic/template types. As seen below, I can change the actual entity to be a generic T and pass the two lines that do not use the common properties among the entities as expressions/methods.
Below is the C# type of pseudocode that demonstrates the ideal solution but I don't know how to actually do it in C#.
int? MethodToRefactor<T>(DbSet<T> entity, Expression<Func<T, T> filterMethod,
Expression<Func<T, T> getIdMethod, List someCollection, string[] moreParams) where T : Account, Customer //This will fail
{
int? keyValue = null;
foreach (var itemDetail in someCollection)
{
string refText = GetRefTextBySource(itemDetail, moreParams);
if (filterMethod(entity) == true)
keyValue = getIdMethod(entity);
if (...some conditional code...)
break;
}
return keyValue;
}
void Caller()
{
foreach (var entity in EntityCollection)
{
if (entity.Name == "Account")
{
id = MethodToRefactor<Account>(db.Accounts, () => {entity.Count(a => a.Name == refText) > 0}, () => {entity.Where(a => a.Name == refText).First().AccountId},...);
}
else if (entity.Name == "Customer")
{
id = MethodToRefactor<Customer>(db.Customer, () => {entity.Count(c => c.CustomerName == refText) > 0}, () => {entity.Where(c => c.CustomerName == refText).First().CustomerId},...);
}
}
}
Benefits / Goals Achieved 1. We combined all of MethodToRefactors into one and eliminated all duplicate code. 2. We abstracted away entity specific operations to the Caller. This is important because that logic is moved to the one logical place that knows how different entities differ from each other (Caller had a per entity ifelse to begin with) and how those differences are to be used. 2. By delegating the entity specific code to the Caller we also made it more flexible so that we don't have to create one MethodToRefactor per entity specific logic.
Note: I'm not a big fan of Adapter, Strategy etc, I prefer solutions that can achieve those goals using C# language features. That doesn't mean I'm anti-classical-design-patterns, it's just that I don't like the idea of creating a bunch of new classes when I can do it by refactoring into a couple of methods.
If the entities do not have the same base class, the best you can do is to have a class constraint.
Since both expressions are essentially the same, you should just pass one expression and a function to get the key value from the entity.
The Count and First methods can also be merged into a single statement and then checking for null.
int? MethodToRefactor<T>(DbSet<T> entities, Func<string, Expression<Func<T, bool>>> expressionFilter, Func<T, int> getIdFunc, IList<string> someCollection, string[] moreParams)
where T : class
{
int? keyValue = null;
foreach (var itemDetail in someCollection)
{
string refText = GetRefTextBySource(itemDetail, moreParams);
var entity = entities.FirstOrDefault(expressionFilter(refText));
if (entity != null)
{
keyValue = getIdFunc(entity);
}
if (...some conditional code...)
break;
}
return keyValue;
}
You would call the method like this
id = MethodToRefactor<Account>(db.Accounts, txt => a => a.Name == txt, a => a.AccountId, ...);
id = MethodToRefactor<Customer>(db.Customers, txt => c => c.CustomerName == txt, c => c.CustomerId, ...);