CODEWITHSUNDEEP
coding-stylesingle-responsibility-principle
Tesserex
Tesserex

Reputation: 17314

Method-level SRP vs. interface bloat

I suggested a refactoring a coworker and he countered basically quoting the SRP. Here's the situation.

We have a bunch of helper methods that to me are all a related purpose - html generation. There could be a lot of options to apply, let's call them A B and C, that you can mix and match.

His original code used a separate method for every option, and for the valid combinations. I saw this as bad because the permutations can quickly escalate out of control.

public string MethodWithA() { /* ... */ }

public string MethodWithB() { /* ... */ }

public string MethodWithC() { /* ... */ }

public string MethodWithAandB() { /* ... */ }

public string MethodWithAndC() { /* ... */ }

public string MethodWithBandC() { /* ... */ }

Our situation isn't quite as extreme as this, but I'm asking for the general case.

I said there should be a single method and the options should be passed as parameters or enum flags.

public string Method(SomeOptions flags)
{
    /* minimal base processing */

    if (/* flag A */)
    {
        ModifyForA();
    }

    /* etc for B and C */
}

His response was that switching on flags like that means the method is doing multiple things. I know "Clean Code" does say something about flags or switch statements being a smell, but I don't think it applies to this case. I think that rule is about finding opportunities for polymorphism. In any case, I think that my version is still doing one thing, but I guess it's up to interpretation.

Is there anything not entirely subjective that we can use to resolve which approach is better?

Upvotes: 1

Views: 95

Answers (1)

Zdeslav Vojkovic
Zdeslav Vojkovic

Reputation: 14581

It is hard to tell as your examples are a bit too generic, but I don't like either of these approaches. The first one results in explosion of combinations, as you correctly note, but the alternative will result in a monstrous method which is impossible to test or analyze. I would prefer some kind of builder, or as it is popular now to call it fluent interface. Then you would have something like:

string html = htmlBuilder.WithA().WithC().Build()

Upvotes: 1

Related Questions