Helper class(es) vs functional inheritance. Best practice

Question

I have commands classes that implement ICommand { Execute } interface. Several commands have duplicate pieces of code. I have several options how to DRY:

• Create static helper class(es) and move duplicate code there
• Create commands inheritance with protected helper methods

What would you suggest and why?

ADDED Thank you everyone who replied, many answers were alike and useful!

Answer 1

If there's any chance that the duplicate logic might be needed by other classes outside the hierarchy as well, I'd put it into static helper classes. Otherwise in the base class with protected inheritance.

Answer 2

In my opinion, I would put the duplicate code into the base class if it only relates to that class hierarchy and will not be used outside of it. If there is any possibility for the code to be used across different classes then move it to a helper class within a common project.

Enjoy!

Answer 3

Instead of static classes another option is to put the common code in a new class and use dependency injection to inject the helper class into the commands. This also goes with the composition over inheritance notion as well.

Answer 4

There is probably not a right/wrong answer here, though I suppose you could certainly implement it poorly. It's likely very dependent on your actual requirements and how related your commands are to one another. Generally, I'd probably go with a base class implementation and inheritance hierarchy, assuming that the commands are related and the code relates directly to the commands themselves, not to some external entity which should be a class in its own right. Certainly they are related by the fact that they are commands and the base class could reflect that.

If you have code that is only common to subsets of unrelated commands, though, and creating an inheritance hierarchy would be forcing a relationship that doesn't exist, then adding in a "helper" class (non-static, if possible, to improve testability) would be a perfectly natural way to address the issue. You may find that you can group the "helper" methods naturally into classes of their own. For example, if several methods need to interact with your authentication subsystem, you might have an AuthenticationMediator for those methods. I also don't see any conflicts with doing some of both.

Answer 5

If the logic operates on interface members, as opposed to implementation members, then writing a helper method [or extension method] is advised.

public IRandom
{
    byte NextByte ();
}

public static class IRandomExtensions
{
    // logic that acts against public interface IRandom
    // may be applied to all implementations
    public static int GetNextUnbiasedInteger (this IRandom random) { }
    public static IEnumerable<T> Shuffle<T> (
        this IRandom random, 
        IEnumerable<T> items) { }
}

If business logic operates against implementation members,

public class SomeCommand : ICommand
{
    // an implementation-specific member, NOT a member
    // of ICommand
    public int SomeControlCount { get; set; }
}

// a method that references implementation-speciic
// details. where should this go?
public void DoSomething ()
{
    SomeCommand command;
    int count = command.SomeControlCount;
}

then likely we should more tightly bind this to the implementing class. If this is a common enough occurrence, then a base class may make sense.

Personally, complex hierarchies are more trouble than they are worth, use your own judgement with respect to maintainability, legibility, and reuse, and you should be ok!

Hope this helps! :)

Answer 6

It completely depends on the nature of your duplicated code.

What are the inputs / outputs of you helper functions? Do they operate on a logically related set of variables? Then - yes, you'd better create a base class with those variables as members, and related set of helper functions.

Otherwise, if the parameters in your helper functions are not coherent, you would implement those functions as static functions anyway, right? I don't see reasons to complicate things with inheritance in this case and I would do it just with helper functions (or, if your language doesn't treat functions as first class citizens, use static helper class).