C# – Refactoring Abstract Classes into Helper Classes

abstract classc

A lot of my code seems to be going through the following evolution –

Iteration 1

interface IAccountManager {
    void Import(string importData);
}
class SalesAccountManager : IAccountManager {
    private int GetAccountId(string data) { 
        //Some data conversions, database references, calculations etc
        return accountId;
    }
    private int GetAccountName(string data) { 
        //Some data conversions, database references, calculations etc
        return name;
    }
    public void Import(string importData) {
        SalesAccount a = new SalesAccount();
        a.Id = GetAccountId(importData.Substring(11, 55); //hardcoding just to keep this example simple
        a.Name = GetAccountName(importData);
        a.Save();
    }
}

It all works well, everyone is happy but then I find I need to add a BusinessAccountManager. Upon further inspection, it seems that the GetAccountId and GetAccountName methods are not going to change. So, what seems like a natural solution? We just make an Abstract base class that inherits from IAccountManager and make those methods protected and make Import abstract. Now, we have –

Iteration 2

abstract class AbstractAccountManage : IAccountManager {
    protected int GetAccountId(string data) {...No change here...}
    protected int GetAccountName(string data) {...No change here...}
    public abstract void Import(string importData);
}

class SalesAccountManager : AbstractAccountManager {
    public void Import(string importData) {...No change here...}
}
class BusinessAccountManager : AbstractAccountManager {
    public void Import(string importData) {
        BusinessAccount a = new BusinessAccount();
        a.Id = GetAccountId(importData.Substring(11, 55); //hardcoding just to keep this example simple
        a.Name = GetAccountName(importData);
        a.Save();
    }
}

But it seems this abstract class is basically a helper class, so I could've done the following –

Iteration 3

interface IAccountManagerStrategy {
    public int GetAccountId(string data);
    public int GetAccountName(string data);
}
class AccountManagerStrategy : IAccountManagerStrategy {
    public int GetAccountId(string data) {...No change here...}
    public int GetAccountName(string data) {...No change here...}
}
interface IAccountManager {
    void Import(string importData);
}

class SalesAccountManager : IAccountManager {
    private IAccountManagerStrategy accountManagerStrategy {get;}
    public SalesAccountManager(IAccountManagerStrategy accountManagerStrategy) {
        this.accountManagerStrategy = accountManagerStrategy;
    }
    public void Import(string importData) {
        SalesAccount a = new SalesAccount();
        a.Id = accountManagerStrategy.GetAccountId(importData.Substring(11, 55); //hardcoding just to keep this example simple
        a.Name = accountManagerStrategy.GetAccountName(importData);
        a.Save();
    }
}

class BusinessAccountManager : AbstractAccountManager {
    //Similar to above but for a BusinessAccount        
}

Note: The implementation of GetAccountId and GetAccountName methods is not very likely to change (but one never knows).

With the above background –

Questions

  1. From a software architecture perspective, is it a good approach to refactor such abstract classes that act as repositories of shared private methods (similar to above) into helper classes (or strategy pattern, if we want to call it that) as seen in Iteration 3? This question is not for this specific case but in general.

  2. If it is a good approach to do that refactoring, then what is the real use of abstract classes and the protected keyword as far as software design/architecture is concerned?

  3. Would an abstract class like AbstractAccountManager above be violating SRP after the methods were changed from private to protected (I suppose it can be argued that its no longer doing only one thing, but difficult to say when to stop identifying what a single responsibility should be)?

Note: This question is not about composition vs polymorphism. It is about understand the correct usage of abstract and understanding why or why not refactor concrete methods of an abstract class into helper/utility classes.

Best Answer

From a software architecture perspective, is it a good approach to refactor such abstract classes that act as repositories of shared private methods (similar to above) into helper classes (or strategy pattern, if we want to call it that) as seen in Iteration 3? This question is not for this specific case but in general.

In general this is a reasonably good idea. Both helper classes and an abstract base class take the burden of knowing how to implement a certain bundle of functionality from a class, and hence allow us to consolidate shared functionality. The advantage of a helper class is that it allows us to change that functionality more dynamically and independently, we can replace the helper with a mock/stub during testing, inject different implementations of the helper and change two different dependents on the helper independently without changing their interfaces.

An abstract class has the advantage of convenience: it requires fewer keypresses to delegate functionality to a base class than a helper class. This is not a significant benefit. And when the abstract base class is just providing some private methods, its main advantages -- essentially serving as a template or transformation from interface to interface -- are not leveraged. So I think overall this is a solid idea.

If it is a good approach to do that refactoring, then what is the real use of abstract classes and the protected keyword as far as software design/architecture is concerned?

There are frequently cases where some functionality can be defined as a wrapper around some smaller set of functionality. There are a few subcases of this:

  1. Checks around unsafe operations. Suppose I have a stack that should throw a IllegalOperationException when it is empty and pop is called. I can use an abstract class to abstract away these safety checks -- thus ensuring that child classes cannot forget to implement them. This can look like

    public abstract class SafeStack<T> {
    protected abstract T UnsafePop();
    protected abstract int Size { get; }

    public T Pop() {
        if (this.Size <= 0) {
            throw new IllegalOperationException("Cannot pop an empty stack.");
        }
        return this.UnsafePop();
    }
}

  2. "Template classes". Sometimes you get into cases where you have a number of classes that implement some common functionality by the same recipe. So I might have

    public class ExcelReportGenerator {
    private void SaveToExcelFile(Summary summary){}

    public void SaveReport(ReportData data) {
        CleanReportData cleaned = data.Clean();
        Summary summary = cleaned.Summary();
        SaveToExcelFile(summary);
    }
}

public class HtmlReportGenerator {
    private void SaveToHtmlFile(Summary summary){}

    public void SaveReport(ReportData data) {
        CleanReportData cleaned = data.Clean();
        Summary summary = cleaned.Summary();
        SaveToHtmlFile(summary);
    }
}

    You can remove the duplication by making an abstract ReportGenerator with a protected abstract SaveToFile(Summary) method.

  3. Lots of delegation. Suppose you have classes implementing some ComplicatedMethod(ComplicatedParameters parameters) where the parameters have many optional parameters / reasonable defaults. If you want to define methods that delegate to the ComplicatedMethod(ComplicatedParameters) with various defaults set, an abstract base class with abstract ComplicatedMethod(ComplicatedParameters) with the delegating methods defined there is a reasonably nice way to accomplish this.

Now, you do not have to use abstract classes for any of these. You can always simulate an abstract class by defining an interface for the child class and taking that interface as a constructor parameter. And over the last decade or so mainstream languages have defined other nice ways for accomplishing the same thing, such as generic classes, extension methods or interface defaults.

I think there are still cases where you're trying to accomplish some mix of the above three where the niceties of using an abstract base class can overpower the downsides of inheritance to make it worthwhile.

Would an abstract class like AbstractAccountManager above be violating SRP after the methods were changed from private to protected (I suppose it can be argued that its no longer doing only one thing, but difficult to say when to stop identifying what a single responsibility should be)?

This is an interesting thought, and maybe? As you note, "responsibility" is a hard thing to really pin down, but in general going from private to protected is adding to your interface. A non-sealed class has three interfaces: the one it shows to the world (public) the one it shows to its its children (public and protected) and the one it shows to its overriders (virtual). If you think of responsibility as being tied to a class's interface, which is about right, then adding a protected member is growing the responsibility.

Bonus, unasked question. It looks to me like your example is crying out for a generic implementation. Something like

class AccountManager<TAccount> : IAccountManager where T : IAccount, new() {
    private int GetAccountId(string data) { 
        //Some data conversions, database references, calculations etc
        return accountId;
    }
    private int GetAccountName(string data) { 
        //Some data conversions, database references, calculations etc
        return name;
    }
    public void Import(string importData) {
        TAccount a = new TAccount();
        a.Id = GetAccountId(importData.Substring(11, 55); //hardcoding just to keep this example simple
        a.Name = GetAccountName(importData);
        a.Save();
    }
}
Related Topic