I'm currently working on refactoring the code-base for one of our services. I'm been going through reviewing everything, and I feel it's a bit scattered, and could probably adhere to OOP principles better.
I have three classes that are all derivative of another class Cache
. All three of these classes perform the exact same operations, the only difference is the type of object they are querying for and what method call they use to query them. What would be the best method of making these classes even more simple?
public static class ZenDeskCache
{
public static ZendeskApi Api = new ZendeskApi(GlobalVariables.ZendeskUrl, GlobalVariables.ZendeskUser,
GlobalVariables.ZendeskPass);
public class Users : Cache<Users, List<User>>
{
protected override List<User> GetData()
{
var users = Api.Users.GetAllUsers();
var allUsers = new List<User>(users.Users);
while (users.NextPage != null)
{
users = Api.Users.GetByPageUrl<GroupUserResponse>(users.NextPage);
allUsers.AddRange(new List<User>(users.Users));
}
allUsers = allUsers.OrderBy(n => n.Name).ToList();
return allUsers;
}
protected override TimeSpan GetLifetime()
{
return TimeSpan.FromDays(1);
}
}
public class Organizations : Cache<Organizations, List<Organization>>
{
protected override List<Organization> GetData()
{
var organizations = Api.Organizations.GetOrganizations();
var allOrgs = new List<Organization>(organizations.Organizations);
while (organizations.NextPage != null)
{
organizations = Api.Users.GetByPageUrl<GroupOrganizationResponse>(organizations.NextPage);
allOrgs.AddRange(new List<Organization>(organizations.Organizations));
}
allOrgs = allOrgs.OrderBy(n => n.Name).ToList();
return allOrgs;
}
protected override TimeSpan GetLifetime()
{
return TimeSpan.FromDays(1);
}
}
public class Groups : Cache<Groups, List<Group>>
{
protected override List<Group> GetData()
{
var groups = Api.Groups.GetGroups();
var allGroups = new List<Group>(groups.Groups);
while (groups.NextPage != null)
{
groups = Api.Groups.GetByPageUrl<MultipleGroupResponse>(groups.NextPage);
allGroups.AddRange(new List<Group>(groups.Groups));
}
allGroups = allGroups.OrderBy(n => n.Name).ToList();
return allGroups;
}
protected override TimeSpan GetLifetime()
{
return TimeSpan.FromSeconds(600);
}
}
}
Best Answer
The problem is that the design of the underlying API loves to repeat type information in all property names:
This misdesign makes it difficult to abstract over all otherwise identical
Api.X
objects. In particular, your methods differ in these points:Api.X
object that methods are called onApi.X.GetX()
method to get the inital responsex.X
that accesses the data in one responseGetPageByUrl
The solution is to introduce an appropriate layer that levels away these differences. E.g. you could define a common function
CommonGetData
that could be invoked like this:[1][1]: Please note that I'm not fluent in C#, so only take note of the conceptual design rather than the specifics of syntax.
With
CommonGetData
being defined approximately like this:If using lambdas is not your thing, you could also note that this looks remarkably like a candidate for either the Strategy Pattern (in
CommonGetData
, the callback parameters represent strategies), or for the template method pattern. We could then define an abstract Api like this:which would be used like
Ideally, the existing APIs could be refactored to implement common interfaces, so that wrapping them in common interfaces that delegate to the incompatible methods would not be necessary.
Also note that your very high use of static classes probably gets in the way of code reuse, since it makes it difficult to pass these around as values. If only one instance may exist, prefer the Singleton Pattern which also makes your classes easier to test when compared with static classes.