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 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 two of our best selling eBooks for FREE!

JPA Mini Book

Learn how to leverage the power of JPA in order to create robust and flexible Java applications. With this Mini Book, you will get introduced to JPA and smoothly transition to more advanced concepts.

JVM Troubleshooting Guide

The Java virtual machine is really the foundation of any Java EE platform. Learn how to master it with this advanced guide!

Given email address is already subscribed, thank you!
Oops. Something went wrong. Please try again later.
Please provide a valid email address.
Thank you, your sign-up request was successful! Please check your e-mail inbox.
Please complete the CAPTCHA.
Please fill in the required fields.

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


× eight = 40



Java Code Geeks and all content copyright © 2010-2014, Exelixis Media Ltd | Terms of Use | Privacy Policy | Contact
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.
Do you want to know how to develop your skillset and become a ...
Java Rockstar?

Subscribe to our newsletter to start Rocking right now!

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

Get ready to Rock!
You can download the complementary eBooks using the links below:
Close