Avoiding FORs – Anti-If Campaign

Have you ever wondered how FORs impact your code? How they are limiting your design and more important how they are transforming your code into an amount of lines without any human meaning?

In this post we are going to see how to transform a simple example of a for (provided by Francesco Cirillio – anti-if campaign), to something more readable and well designed.

So let’s start with original code using FOR:
 
 
 

 public class Department {

  private List<Resource> resources = new ArrayList<Resource>();

  public void addResource(Resource resource) {
   this.resources.add(resource);
  }

  public void printSlips() {

   for (Resource resource : resources) { 
    if(resource.lastContract().deadline().after(new Date())) { 

     System.out.println(resource.name()); 
     System.out.println(resource.salary());
    }
   }
  }

 }

See printSlips method. So simple method, only 10 lines counting white lines, but violating one of the most important rule, this method is doing more than one thing inside it mixing different level of abstractions.

As Robert C. Martin note in his book ‘Functions Should Do One Thing. They Should Do Well. They Should do it only […]. If a function does only steps that are one level below the stated name of the function, then the function is doing one thing […].’.

So with the given definition of how a method should look, let’s recap with previous method, and see how many things are doing
printSlips method? Concretely four.

 public void printSlips() {

   for (Resource resource : resources) { #Cycle
    if(resource.lastContract().deadline().after(new Date())) { #Selection
     #Media 
     System.out.println(resource.name()); #Content
     System.out.println(resource.salary());
    }
   }
 }

The method is cycling, selecting resources, accessing to content, and accessing to media. See that each of them belongs to different level of abstraction, printing to console should be in different level of checking if a resource has not expired its contract yet.

Let’s see the solution proposed by Francesco.

The first thing to do is splitting main functions into three classes and two interfaces, one for iterating resources, another one to choose whatever a resource has not expired yet, and another one for printing a resource. With this approach, we are creating a solution which is designed to be extended, and improving readability too.

And now it is time for the code:

Predicate interface will be used for implementing if a resource meets an implemented condition.

 public interface Predicate {

  boolean is(Resource each);

 }

For example in our case, implementation of interface will look like:

 public class InForcePredicate implements Predicate {

  public boolean is(Resource each) {
   return each.lastContract().deadline().after(new Date());
  }
 }

We have move conditional to InForcePredicate class. Note that if we want to create a class that checks if contract is expired we would create a new class implementing Predicate with something like return  each.lastContract().deadline().before(new Date());

Block interface will be the next interface which will implement the access to media. In this case to console:

 public interface Block {

  void evaluate(Resource resource);

 }

And its implementation:

 public class PrintSlip implements Block {

  public void evaluate(Resource resource) {
   System.out.println(resource.name()); 
   System.out.println(resource.salary());
  }

 }

Again note that changing where information is sent (console, file, network, …) it is a simply matter of implementing Block interface.

And last class is the one which contains an iterator over resources, and also provides methods to call each interface created previously:

 public class ResourceOrderedCollection {
  private Collection<Resource> resources = new ArrayList<Resource>();

  public ResourceOrderedCollection() {
   super();
  }

  public ResourceOrderedCollection(Collection<Resource> resources) {
   this.resources = resources;
  }

  public void add(Resource resource) {
   this.resources.add(resource);
  }

  public void forEachDo(Block block) {
   Iterator<Resource> iterator = resources.iterator();

   while(iterator.hasNext()) {
    block.evaluate(iterator.next());
   }

  }

  public ResourceOrderedCollection select(Predicate predicate) {

   ResourceOrderedCollection resourceOrderedCollection = new ResourceOrderedCollection();

   Iterator<Resource> iterator = resources.iterator();

   while(iterator.hasNext()) {
    Resource resource = iterator.next();
    if(predicate.is(resource)) {
     resourceOrderedCollection.add(resource);
    }
   }

   return resourceOrderedCollection;
  }
 }

See next three important points:

• First one is that constructor receives a list of resources.
• Second one is that select method receives a predicate which is executed into the iterator to know if resource is choosable to be printed or not. Finally returning a new instance of ResourceOrderedCollection with resources without an expired contract.
• Third one forEachDo method receives a Block interface which is called by every element of resources list.

And finally modified Department class using previous developed classes:

 public class Department {

  private List<Resource> resources = new ArrayList<Resource>();

  public void addResource(Resource resource) {
   this.resources.add(resource);
  }

  public void printSlips() {
   new ResourceOrderedCollection(this.resources).select(new InForcePredicate()).forEachDo(new PrintSlip());
  }

 }

Observe that now printSlips method contains a single readable line with the same level of abstraction.

Take notice that class and interface names are taken from Francesco example, but if I should do the same I would choose more representative names. Cirillo’s approach is good, but has some minor aspects to consider. For example it has the ‘ vertical problem‘: the InForcePredicate instance from Predicate interface uses five lines of source code to encapsulate a single statement.

We have explored two possible solutions of a problem, being the last one the proposed by Cirillio. Also there are many other possible and correct solutions to this problem, for example using Template Method Pattern, or mixing the use of Lambdaj with (or without) Closures (Lambdaj syntax can be a bit confusing). All of them have their pros and cons, but all of them makes your code readable and more important, all functions do one thing, they do well and they do it only.

As final notes of this post, JDK 8 will provide support for closures natively, and also will provide many features that now are provided by Lambdaj. Meanwhile JDK 8 is not stable (planned for mid-final 2013) or for your legacy code (from the point of view of JDK 8), Lambdaj is a really good fellow traveler.

We keep learning.
 

Reference: Avoiding FORs – Anti-If Campaign from our JCG partner Alex Soto at the One Jar To Rule Them All blog.

Do you want to know how to develop your skillset to become a Java Rockstar?

Subscribe to our newsletter to start Rocking right now!

To get you started we give you our best selling eBooks for FREE!

1. JPA Mini Book

2. JVM Troubleshooting Guide

3. JUnit Tutorial for Unit Testing

4. Java Annotations Tutorial

5. Java Interview Questions

6. Spring Interview Questions

7. Android UI Design

and many more ....

I agree to the Terms and Privacy Policy

Sign up

Thank you!

We will contact you soon.