Implementation of the Strategy pattern

Question

I am learning the Strategy pattern but hate the idea of passing a new object to the Context each time I need to change the desired algorithm. Instead I think it would be best to create an enum that holds all of the Concrete Strategy objects, and simply update them with a setter on the context. Below is my implementation. Please let me know if this is a good approach for Strategy?

Client

public class Client {
 public static void main(String args[]){
 IntroductionContext introductionContext = new IntroductionContext();
 introductionContext.setStrategy(ConcreteStrategies.HELLO);
 introductionContext.executeStrategy();
 introductionContext.setStrategy(ConcreteStrategies.GOODBYE);
 introductionContext.executeStrategy();
 }
}

Context

public class IntroductionContext {
 private IInteractionStrategy iInteractionStrategy;
 public void executeStrategy() {
 if(iInteractionStrategy == null){
 throw new RuntimeException("Context must set its strategy before invoking executeStrategy()");
 }
 this.iInteractionStrategy.interact();
 }
 public void setStrategy(ConcreteStrategies concreteStrategy){
 this.iInteractionStrategy = (IInteractionStrategy) concreteStrategy.getConcreteStrategy();
 }
}

Enum to hold all strategy objects

public enum ConcreteStrategies {
 HELLO(new HelloStrategy()), GOODBYE(new GoodByeStrategy());
 private Object concreteStrategy;
 private ConcreteStrategies(Object concreteStrategy){
 this.concreteStrategy = concreteStrategy;
 }
 public Object getConcreteStrategy(){
 return this.concreteStrategy;
 }
}

Strategy (Interface)

public interface IInteractionStrategy {
 public void interact();
}

Concrete Strategy

public class HelloStrategy implements IInteractionStrategy {
 @Override
 public void interact() {
 System.out.println("Hello!");
 }
}

Concrete Strategy

public class GoodByeStrategy implements IInteractionStrategy {
 @Override
 public void interact() {
 System.out.println("Goodbye!");
 }
}

• shivsky: Is ConcreteStrategies an enum or a class?
• Simon Forsberg: @shivsky It's an enum. In Java, enums can contain methods.

Answer 1

The overall quality of your code is very good. It is, at least in my opinion, perfectly fine to have an enum containing the strategy implementations.

A few comments and suggestions though:

• private Object concreteStrategy; in your enum can and should be private IInteractionStrategy concreteStrategy; instead. No need to use Object references at all in your enum when you can use IInteractionStrategy instead. By doing this, you won't have to typecast in your setStrategy method.
• After implementing the above, private IInteractionStrategy concreteStrategy can and should be marked final.
• Even though it's fine to store all your strategies in your enum, you shouldn't enforce it. By having the method public void setStrategy(ConcreteStrategies concreteStrategy) you deny any strategies that are not part of your enum. Either change this method, or add a new one, to be public void setStrategy(IInteractionStrategy concreteStrategy)
• IntroductionContext could include a executeStrategy(IInteractionStrategy) method to execute a specific strategy. Then you don't need to first call setStrategy and then call executeStrategy (This is only a suggestion, use it if you like)

• Shijima: Very nice analysis! Thanks for your detailed review. I was actually thinking that Object in the enum was a bad choice after I posted the code, I will definitely update that. On your point 3, I do want to enforce the context setter to only take enums. The reason for this is, from the client perspective, it will eliminate needing to know ALL of the possible concrete strategies available, as they will all be easily accessible through the ConcreteStrategies enum. Although I guess a better name for that enum might be needed. Thanks again!

Answer 2

I'll add a little Java naming advice to complement Simon's thorough answer.

• Enum names are typically singular since you are always referencing a single value: ConcreteStrategy.HELLO instead of ConcreteStrategies.HELLO.

• I'm sure some people still prefix their interface names with I, but I cannot remember the last project I've worked on or open source tool I've used that does. That smells like leaking an implementation detail into the API.

  While it seems contradictory, it's still common to include Abstract in the names of abstract methods. I do this too because a) you have to have a different name from the interface and b) the abstract class is for implementers and doesn't go into the public API (normally).

• If you keep the I prefix, I would still remove the i prefix on the instance field: interactionStrategy instead of iInteractionStrategy.

• Simon Forsberg: "That smells like leaking an implementation detail into the API", whether a type is a class or an interface is a part of the public API. I also haven't seen many interfaces that gets prefixed like that though. Overall, I totally agree. +1.
• David Harkness: @Simon - In a way yes, but I think that matters mostly when the client is required to extend it. If you're returning instances of the class/interface only for consumption, the client doesn't need to know.
• Simon Forsberg: That's true, David. But there's no way to protect the client from being able to know if something is a class or an interface.