eek
eek

Reputation: 47

Avoiding ifs using patterns

I know that you can remove if blocks by using polymorphism with Strategy or Command patterns. However, let's say my code looks something like this:

ArgumentObject arg = getArg();

if (arg.getId() == "id2_1" || arg.getId() == "id3_1" || arg.getId() == "id4_1") {
    //do something
}
if (arg.getId() == "id2_1") {
    //do something
}
if (arg.getId() = "id2_2" || arg.getId() = "id3_1") {
    //do something
}
if (arg.getId().startsWith("id1") || arg.getId().startsWith("id4")) {
    //do something
}

Basically, the code performs some fields population actions based on product's id. It checks whether product comes from some group of products with startsWith(), then performs population, checks if a product has a particular id, then populates fields appropriately, etc.

I am not sure how can I use Strategy pattern here? If I create one Strategy to handle first if statement, then how can I proceed handling the second statement? The problem is that the ifs aren't mutually exclusive. Should I just move the relevant ifs to Strategy classes? Any pattern that would help me refactor this so its easier to test?

Upvotes: 0

Views: 272

Answers (3)

slim
slim

Reputation: 41223

You may not be able to get rid of your if based code entirely, but you may be able to tidy it up into a more appropriate class.

The ifs are domain logic, and if it makes sense within the domain then you could write an object solely responsible for this classification:

 interface ArgumentClassifier {
      Classification classify(Argument arg);
 }

Classification would be an enum.

The implementation of this will contain your ifs, encapsulating that domain logic.

  • You can unit-test your ArgumentClassifier in isolation.
  • If your ArgumentClassifier turns out to be large and complicated, you might well be able to break it up using the Composite Pattern.
  • If you later find a different way to do the classification (e.g. a rules engine, a database lookup, etc.) you can write new implementations of ArgumentClassifier.

Now you can use the Strategy pattern:

  Map<Classification, Handler> strategies = ...;
  ArgumentClassifier classifier = new SimpleArgumentClassifier();

  private Foo handleArg(Argument arg) {
      return strategies
          .get(classifier.classify(arg))
          .handle(arg);  
  }

Whether all this is worthwhile or not, depends on your specific needs.

If:

  • The class you're working in is already the natural home of that domain logic
  • The class you're working on has good cohesion

... then it may be wiser to leave the if logic where it is.


Another approach to classification is to build a binary number or a string from predicates about the object being classified. For example:

 int classification(Person p) {
     return 0
         | ( p.getGender() == Gender.MALE ? 1 : 0 )
         | ( p.isEmployed()               ? 2 : 0 )
         | ( p.age >= 18                  ? 4 : 0 );
 } 

This gives you a classification of 0b001 for an unemployed 17-year-old male, 0b010 for an employed 17-year-old female, and so on.

You can use these directly as keys to a map of strategies (it could be many-to-one). Or you can do further bitwise manipulation to reduce them to a smaller set.

There's a lot of scope here, but take care not to lose sight of simplicity and clarity. Sometimes a chain of if/else gets the job done, is clear and effective.

Upvotes: 0

k1133
k1133

Reputation: 325

Your question lacks the business logic for these ifs. But I can make some assumptions from :

Basically, the code performs some fields population actions based on product's id. It checks whether product comes from some group of products with startsWith(), then performs population, checks if a product has a particular id, then populates fields appropriately, etc.

I am assuming all you if blocks are specific business logic and are independent of each other. You can try Decorator Pattern for this. As the wiki says:

Decorator Pattern is a design pattern that allows behavior to be added to an individual object, either statically or dynamically, without affecting the behavior of other objects from the same class

Your case can be viewed as :

public Product{

    private String id;
    //other fields
}


   ProductDecoratorInterface{

       Product populateItem(Product product);

    }

ProductXYZDecorator{

   @Override
   public Product populateItem(Product product){
   // check the id for 1,2,3
   //do the field population

}

}

    ProductABCDecorator{

    @Override
    public Product populateItem(Product product){
    // check the id for 1,3
    //do the field population

    }

    }

Your main class would look like:

public Mainclass{

List<ProductDecoratorInterface> decorators;


public void someFn(Product product){
   decorators.stream.forEach(i -> i.populateItem(product));
}

}

}

...

Any pattern that would help me refactor this so its easier to test?

Here you would need to test just the individual Decorators.

You would need to maintain the list of all decorator and pass your object through all of them. This is not the proper use of the pattern but helps solve your use case(testing complexity).

Upvotes: 0

Timothy Truckle
Timothy Truckle

Reputation: 15622

The problem your code reveals is somewhat more basic: It violates the Tell! Don't ask! principle.

Instead of having a method getId()your argobjects should have a common interface method which is called at this point. The work to be done could be injected to the arg objects when they are instantiated, or they could be passed as Parameters to the method and the concrete Inmplementation of the arg object decides which parameter to call...

Upvotes: 1

Related Questions