aiquita
aiquita

Reputation: 19

Is this an anti pattern?

Currently I am confronted often with code that follows the pattern demonstrated by the code bellow. It is kind of a strategy pattern.

I am not fine with this, it feels somehow smelly. It breaks the actual strategy pattern and the additional indirection is confusing. Also it leads often to methods called similiar like in the example, because the purpose of the method in the derived class is very similar to the in the base class. On the other side I am not able to point with the finger to the problem core.

Am I the onlyone who finds this fishy? And if not, what problems can this code cause, esepecially with regard to SOLID principles?

namespace MyTest
{
    abstract class Base
    {
        public void DoSomething()
        {
            var param = Prepare();
            DoItReally(param);
        }

        private string Prepare()
        {
            return "Hallo Welt";
        }

        protected abstract void DoItReally(string param);
    }

    class DerivedOne : Base
    {
        protected override void DoItReally(string param)
        {
            Console.WriteLine(param);
        }
    }

    class DerivedTwo : Base
    {
        protected override void DoItReally(string param)
        {
            Console.WriteLine(param.ToUpper());
        }
    }
    
    static class Program
    {
        public static void Main(string[] args)
        {
            Base strategy = new DerivedOne();
            strategy.DoSomething();
            
            strategy = new DerivedTwo();
            strategy.DoSomething();
        }
    }
}

Upvotes: 1

Views: 312

Answers (1)

Flater
Flater

Reputation: 13773

If you separate the code into its two constituent parts, you should see that both of them are perfectly fine.

1. Abstracting some logic to a private method

public void DoSomething()
{
    var param = Prepare();
    DoItReally(param);
}

private string Prepare()
{
    return "Hallo Welt";
}

Given that the method bodies for both methods are fixed, and in this case really short, we can argue about the private method not being necessary here. But this is just an example, and in cases where your initialization logic becomes much more complex (e.g. a complex data query and calculation), the need to abstract this into a private method increases.

2. Subclasses implementing abstract base methods

That's exactly why abstract exists as a keyword.

Your base class know how to fetch the data, but since there are multiple ways of handling this data, there are subclasses which each define one of those options. You've maximized reusability in the base class, the only thing that's not reusable is each subclass' custom handling logic.


I think you're getting caught up on labeling patterns and antipatterns much more than is actually productive.

Clean code doesn't come from the quest to find antipatterns. Clean code comes from understanding your needs, realizing that the code does not suit your purpose or has an unwanted side effect, and then refactoring the code to keep its benefit while losing or minimizing its drawbacks.

As of right now, you have not shown any issue with the code itself, or why it may be causing an unwanted side effect. Your question is hypochondriac in nature, you're over-eagerly looking for issues, rather than simply trying to fix an issue that concretely affects you.

Upvotes: 5

Related Questions