Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
    
Hmmmmm ....it might be possible to simplify things with things like eval... might not work in Java but maybe in other languages? –  FrustratedWithFormsDesigner May 1 '12 at 19:14
1  
@FrustratedWithFormsDesigner reflection is the magic word in java –  ratchet freak May 1 '12 at 19:38
1  
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 2 '12 at 8:21
1  
The flaw you are mentioning is often called violation of the open-closed-priciple –  k3b May 2 '12 at 8:54
    
accepted answer in related question suggests dictionary / map as an alternative to if-else and switch –  gnat May 2 '12 at 12:30
add comment

6 Answers

up vote 13 down vote accepted

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.

share|improve this answer
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 1 '12 at 19:34
add comment

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.

share|improve this answer
1  
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 4 '12 at 7:18
2  
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 4 '12 at 7:37
    
I don't see that you've avoided branching either. You've just changed the syntax. –  pdr May 4 '12 at 7: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 4 '12 at 7:56
    
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
show 1 more comment

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.

share|improve this answer
add comment
  • 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. 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 in what state the object is or what type of object you are working with. You will then build a strategy for that. Good old separation of concerns :)

share|improve this answer
add comment

"Strategy" is about having to choose between alternative algorithms once, no more, but also no 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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.