About Alex Soto

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 Cirillioanti-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.

Related Whitepaper:

Bulletproof Java Code: A Practical Strategy for Developing Functional, Reliable, and Secure Java Code

Use Java? If you do, you know that Java software can be used to drive application logic of Web services or Web applications. Perhaps you use it for desktop applications? Or, embedded devices? Whatever your use of Java code, functional errors are the enemy!

To combat this enemy, your team might already perform functional testing. Even so, you're taking significant risks if you have not yet implemented a comprehensive team-wide quality management strategy. Such a strategy alleviates reliability, security, and performance problems to ensure that your code is free of functionality errors.Read this article to learn about this simple four-step strategy that is proven to make Java code more reliable, more secure, and easier to maintain.

Get it Now!  

12 Responses to "Avoiding FORs – Anti-If Campaign"

  1. Yannick Majoros says:

    I’d really want to see an example where replacing for’s with this structure is more reasonable. This is just unrealistic. That’s what you’ll get with closures, just pick another language or wait for it. I’d much rather pick a project with the first example than one with the “better” one ;-)

  2. Dmitriy Shyshchenko says:

    See comments on the original post. The idea is not bad, but this examples are really bad. 1 simple loop is replaced by several classes and methods with little to no benefit at the end.

  3. Tom Lee says:

    I prefer the original example to your proposed improvements. Sure, the original could probably do with a little refactoring to separate the business logic from presentation (for example), but you shouldn’t need to jump through the hoops you’re jumping through here.

    There might be a good point or two somewhere in this article, but it’s hopelessly obfuscated by that example.

  4. lanzz says:

    Your PrintSlip::evaluate method does two of the initial four things you mentioned: accessing content and accessing media. I’m sure you can refactor it into three more classes and at least an interface or two.

  5. Siva Prasad Reddy says:

    Perfect example for “Over Engineering” :-)

  6. foo says:

    A complete class with just the meaning to wrap a foreach? Not really ..

  7. Oliver Weiler says:

    Obviously in this case Java is not the right tool for the job.

  8. Epo Jemba says:

    Hi !

    mhhh all previous comments may be right but the actual purpose of this POC (because this is just a POC) is to show that we can handle language block structure like for or if by choosing the right concept and with a better expressivity.

    OK in this sample the interest might be less visible but I’ve imagined reading the article that people would have understood what the author want to show.

    I have in mind code in projects where numerous for loop are imbricated without readability and without testability.

    What is clear is that java language is a verbose one, but with static import + the right structure I can choose this kind of systems into my own projects.

    Message for Alex S. : in your example the loop actualy occurs 2 times once in select() and once in the forEachDo() perhaps this can be optimised by running the Block inside the select . What do you think ?

    Cheers.

Leave a Reply


three + 5 =



Java Code Geeks and all content copyright © 2010-2014, Exelixis Media Ltd | Terms of Use
All trademarks and registered trademarks appearing on Java Code Geeks are the property of their respective owners.
Java is a trademark or registered trademark of Oracle Corporation in the United States and other countries.
Java Code Geeks is not connected to Oracle Corporation and is not sponsored by Oracle Corporation.

Sign up for our Newsletter

15,153 insiders are already enjoying weekly updates and complimentary whitepapers! Join them now to gain exclusive access to the latest news in the Java world, as well as insights about Android, Scala, Groovy and other related technologies.

As an extra bonus, by joining you will get our brand new e-books, published by Java Code Geeks and their JCG partners for your reading pleasure! Enter your info and stay on top of things,

  • Fresh trends
  • Cases and examples
  • Research and insights
  • Two complimentary e-books