14

The Strategy pattern works well to avoid huge if...else constructs and make it easier to add or replace functionality. However, it still leaves one flaw in my opinion. It seems like in every implementation there still needs to be a branching construct. It might be a factory or a data file. As an example take an ordering system.

Factory:

// All of these classes implement OrderStrategy
switch (orderType) {
case NEW_ORDER: return new NewOrder();
case CANCELLATION: return new Cancellation();
case RETURN: return new Return();
}

The code after this doesn't need to worry, and there is only one place to add a new order type now, but this section of code still isn't extensible. Pulling it out into a data file helps readability somewhat (debatable, I know):

<strategies>
   <order type="NEW_ORDER">com.company.NewOrder</order>
   <order type="CANCELLATION">com.company.Cancellation</order>
   <order type="RETURN">com.company.Return</order>
</strategies>

But this still adds boilerplate code to process the data file - granted, more easily unit testable and relatively stable code, but additional complexity nontheless.

Also, this sort of construct doesn't integration test well. Each individual strategy may be easier to test now, but every new strategy you add is addition complexity to test. It's less than you would have if you hadn't used the pattern, but it's still there.

Is there a way to implement the strategy pattern that mitigates this complexity? Or is this just as simple as it gets, and trying to go further would only add another layer of abstraction for little to no benefit?

Michael K
  • 15,539
  • 9
  • 61
  • 93
  • Hmmmmm ....it might be possible to simplify things with things like `eval`... might not work in Java but maybe in other languages? – FrustratedWithFormsDesigner May 01 '12 at 19:14
  • 1
    @FrustratedWithFormsDesigner reflection is the magic word in java – ratchet freak May 01 '12 at 19:38
  • 2
    You will still need those conditions somewhere. By pushing them to a factory, you are simply complying with DRY, since otherwise the if or switch statement might have appeared in multiple places. – Daniel B May 02 '12 at 08:21
  • 1
    The flaw you are mentioning is often called [violation of the open-closed-priciple](http://en.wikipedia.org/wiki/Open_Closed_Principle) – k3b May 02 '12 at 08:54
  • accepted answer in [related question](http://programmers.stackexchange.com/questions/145632/pattern-for-select-case-on-object-type-of-interface "Pattern for select case on object type (of interface)") suggests dictionary / map as an alternative to if-else and switch – gnat May 02 '12 at 12:30
  • Why not just `return orderType.newInstance();`? If all of your order types implement the factory interface with the `OrderStrategy newInstance()` method, there is no switch statement. – Shadow Man Mar 30 '18 at 00:57

7 Answers7

16

Of course not. Even if you use an IoC container, you will have to have conditions somewhere, deciding which concrete implementation to inject. This is the nature of the Strategy pattern.

I don't really see why people think this is a problem. There are statements in some books, like Fowler's Refactoring, that if you see a switch/case or a chain of if/elses in the middle of other code, you should consider that a smell and look to move it to its own method. If the code within each case is more than a line, maybe two, then you should consider making that method a Factory Method, returning Strategies.

Some people have taken this to mean that a switch/case is bad. This is not the case. But it should reside on its own, if possible.

pdr
  • 53,387
  • 14
  • 137
  • 224
  • 1
    I don't see it as a bad thing either. However, I'm always looking for ways to reduce the amount of code maintaining it would touch, which prompted the question. – Michael K May 01 '12 at 19:34
  • Switch statements (and long if/else-if blocks) are bad and should be avoided if at all possible in order to keep your code maintainable. That said "if at all possible" acknowledges that there are *some* cases where the switch must exist, and in those cases, try to keep it down to a single place, and one that makes maintaining it less of a chore (it's so easy to accidentally miss 1 of the 5 places in the code that you needed to keep in sync when you don't isolate it properly). – Shadow Man Mar 30 '18 at 01:02
10

Can the Strategy pattern be implemented without significant branching?

Yes, by using a hashmap/dictionary where every Strategy implementation registers at. The factory method will become something like

Class strategyType = allStrategies[orderType];
return runtime.create(strategyType);

Every strategy implementation must register a the the factory with its orderType and some information how to create a class.

factory.register(NEW_ORDER, NewOrder.class);

You can use the static constructor for registration, if your language supports this.

The register method does nothing more than adding a new value to the hashmap:

void register(OrderType orderType, Class class)
{
   allStrategies[orderType] = class;
}

[update 2012-05-04]
This solution is much more complex than the original "switch solution" which i would prefer most of the time.

However in an environment where strategies changes often (i.e. price calculation depending on customer, time, ....) this hashmap solution combined with an IoC-Container could be good solution.

gnat
  • 21,442
  • 29
  • 112
  • 288
k3b
  • 7,488
  • 1
  • 18
  • 31
  • 3
    So now you've got a whole Hashmap of Strategies, to avoid a switch/case? How exactly is rows of `factory.register(NEW_ORDER, NewOrder.class);` cleaner, or less a violation of OCP, than rows of `case NEW_ORDER: return new NewOrder();`? – pdr May 04 '12 at 07:18
  • 5
    It is not cleaner at all. It is counter-intuitive. It sacrifices the keep-it-stimple-stupid-principle to improve the open-closed-priciple. The same applies to inversion-of-control and dependency-injection: these are more difficult to understand than a simple solution. The question was "implement ... without significant branching" and not "how to create a more intuitive soltuion" – k3b May 04 '12 at 07:37
  • 1
    I don't see that you've avoided branching either. You've just changed the syntax. – pdr May 04 '12 at 07:48
  • with this solution: if you invent a new strategy you create an new strategy-implementation-class (open) and you don-t have to change the factory-method itself (=closed) because you dont have to add a new switch case. you would-nt implemente a permission system this way. every time you add new user you have to modifiy the `isLoginAllowed` with code `if user='max' and password='12345' then return true`. instead you replace coding a lot of if-s with data. – k3b May 04 '12 at 07:56
  • 1
    Isn't there a problem with this solution in languages that don't load classes until they are needed? The static code won't run until the class is loaded, and the class will never be loaded because no other class refers to it. – kevin cline Jul 27 '12 at 18:20
  • 2
    Personally I like this method, because it allows you to treat your strategies as mutable _data_, instead of treating them as immutable code. – Tacroy Jul 27 '12 at 19:11
  • 1
    This is the correct answer IMHO. It's also a mechanism whereby the strategies can be configured at runtime. – slim Sep 22 '17 at 15:44
2

"Strategy" is about having to choose between alternative algorithms at least once, not less. Somewhere in your program someone has to make a decision - maybe the user or your program. If you use an IoC, reflection, a datafile evaluator or a switch/case construct to implement this does not change the situation.

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
  • I'd disagree with your statement of **once.** Strategies can be swapped at runtime. For example, after failing to process a request using a standard ProcessingStrategy, a VerboseProcessingStrategy could be chosen and the processing rerun. – dmux Jan 21 '20 at 19:03
  • @dmux: sure, edited my answer accordingly. – Doc Brown Jan 21 '20 at 19:18
1

The strategy pattern is used when you're specifying possible behaviors and is best used when designating a handler at startup. Specifying which instance of the strategy to use can be done via a Mediator, your standard IoC container, some Factory like you describe, or simply by using the right one based on context (since often, the strategy is supplied as part of a wider use of the class that contains it).

For this sort of behavior where different methods need to be called based on data, then I'd suggest using the polymorphism constructs supplied by the language at hand; not the strategy pattern.

Telastyn
  • 108,850
  • 29
  • 239
  • 365
1

I think there should be a limit on how many branches are acceptable.

For example, if I have more than eight cases inside of my switch statement, then I re-evaluate my code and look for what I can re-factor. Quite often I find that certain cases can be grouped into a separate factory. My assumption here is that there is a factory that builds strategies.

Either way, you can't avoid this and somewhere you will have to check the state or type of the object you are working with. You will then build a strategy for that. Good old separation of concerns.

jscs
  • 828
  • 9
  • 17
CodeART
  • 3,992
  • 1
  • 20
  • 23
1

In talking with other developers where I work, I found another interesting solution - specific to Java, but I'm sure the idea works in other languages. Construct an enum (or a map, it doesn't matter too much which) using the class references and instantiate using reflection.

enum FactoryType {
   Type1(Type1.class),
   Type2(Type2.class);

   private Class<? extends Type> clazz;

   private FactoryType(Class<? extends Type> clazz) {
      this.clazz = clazz;
   }

   public Class<? extends Type> getTypeClass() {
      return clazz;
   }
}

This drastically reduces the factory code:

public Type create(FactoryType type) throws Exception {
   return type.getTypeClass().newInstance();
}

(Please ignore the poor error handling - example code :))

It's not the most flexible, as a build is still required...but it reduces the code change to one line. I like that the data is separated from the factory code. You can even do things like setting parameters either by using abstract classes/interfaces to provide a common method or creating compile-time annotations to force specific constructor signatures.

Michael K
  • 15,539
  • 9
  • 61
  • 93
  • Not sure what caused this to bump back to the home page but this is a good answer and in Java 8 you can now create Constructor references without reflection. – JimmyJames Feb 21 '17 at 14:30
1

Extensibility can be improved by having each class define its own order type. Then your factory selects the one that matches.

For example:

public interface IOrderStrategy
{
    OrderType OrderType { get; }
}

public class NewOrder : IOrderStrategy
{
    public OrderType OrderType { get; } = OrderType.NewOrder;
}

public class OrderFactory
{
    private IEnumerable<IOrderStrategy> _strategies;

    public OrderFactory(IEnumerable<IOrderStrategy> strategies) // Injected by IoC container
    {
        _strategies = strategies;
    }

    public IOrderStrategy Create(OrderType orderType)
    {
        IOrderStrategy strategy = _strategies.FirstOrDefault(s => s.OrderType == orderType);

        if (strategy == null)
            throw new ArgumentException("Invalid order type.", nameof(orderType));

        return strategy;
    }
}