Continuing our Clean Code discussion, imagine you have a textbook from which you only need information from a single section. Now imagine that textbook has neither chapters, nor sections or paragraphs, and is even lacking punctuation.

How easy would it be for you to find the information you needed? Painful to think about, right? You’d have to wade through a bunch of content you could care less about just to get to what you needed. Similarly…

Code with long functions is like a book without punctuation

Hopefully, you already use functions, but do you use them well? Do you find yourself navigating long functions full of actions, branches and loops traversing many different layers of abstraction? Or does your code act like a table of contents, allowing you to navigate directly to what’s relevant to you?

Too often, I’m left searching for punctuation in the code. Code organized this way makes it extremely hard for other developers to see the full picture. At best, I’m left with out of date comments, as if a chapter were mislabeled and not caught by the editor.

We need to more easily be able to navigate through our code. In short, our code needs short functions.

Let’s take another look at Conway’s Game of Life, introduced last week, this time with an eye on functions. In the game, cells on a 2D grid survive or perish according to a set of rules applied over time:

1. Any live cell with two or three live neighbors lives on to the next generation.
2. Any dead cell with exactly three live neighbors becomes a live cell, as if by reproduction.

Below you can see a class method that calculates the cells of the next generation. Take a moment to read it and understand what it does.

  public Set nextCells() {
       Set newLiveCells = Sets.newHashSet();
       Set allNeighbors = Sets.newHashSet();
       
       for (Cell liveCell : liveCells) {
           Set neighbors = Sets.newHashSet(
               new Cell(liveCell.getX() - 1, liveCell.getY() - 1),
               new Cell(liveCell.getX(), liveCell.getY() - 1),
               new Cell(liveCell.getX() + 1, liveCell.getY() - 1),
               new Cell(liveCell.getX() - 1, liveCell.getY()),
               new Cell(liveCell.getX() + 1, liveCell.getY()),
               new Cell(liveCell.getX() - 1, liveCell.getY() + 1),
               new Cell(liveCell.getX(), liveCell.getY() + 1),
               new Cell(liveCell.getX() + 1, liveCell.getY() + 1)
           );
           
           allNeighbors.addAll(neighbors);
           
           int liveNeighbors = 0;
           for (Cell neighbor : neighbors) {
               if (liveCells.contains(neighbor))
                   ++liveNeighbors;
           }
           
           if (liveNeighbors == 2 || liveNeighbors == 3)
               newLiveCells.add(liveCell);
       }
       
       Set deadNeighbors = Sets.difference(allNeighbors, liveCells);
       
       for (Cell deadCell : deadNeighbors) {
           Set neighborsOfDeadCell = Sets.newHashSet(
               new Cell(deadCell.getX() - 1, deadCell.getY() - 1),
               new Cell(deadCell.getX(), deadCell.getY() - 1),
               new Cell(deadCell.getX() + 1, deadCell.getY() - 1),
               new Cell(deadCell.getX() - 1, deadCell.getY()),
               new Cell(deadCell.getX() + 1, deadCell.getY()),
               new Cell(deadCell.getX() - 1, deadCell.getY() + 1),
               new Cell(deadCell.getX(), deadCell.getY() + 1),
               new Cell(deadCell.getX() + 1, deadCell.getY() + 1)
           );
                   
           int liveNeighborsOfDeadCell = 0;
           for (Cell neighborOfDeadCell : neighborsOfDeadCell) {
               if (liveCells.contains(neighborOfDeadCell))
                   ++liveNeighborsOfDeadCell;
           }
           
           if (liveNeighborsOfDeadCell == 3)
               newLiveCells.add(deadCell);
       }
       
       this.liveCells = Sets.newHashSet(newLiveCells);
       
       return newLiveCells;
   }

Perhaps you’re able to follow that long function. It’s not organized horribly, but with a few comments you could more easily understand how it works (assuming those comments were up to date). Whether or not you can follow every step, I can tell you that it gets the job done. But we shouldn’t settle for code like this.

With a little bit of refactoring we can preserve functionality, but truly decompose this task of calculating the next generation into smaller units of work that can be more easily understood, reused, and tested. With good names and small functions, we can create code that reads like a table of contents:

    public Set getNextGeneration() {
        Set nextGeneration = Sets.newHashSet();
        nextGeneration.addAll(getSurvivors());
        nextGeneration.addAll(getNewborns());
        return nextGeneration;
    }

With this approach, the reader can understand what the function is doing in a matter of seconds, and drill down into the details relevant to them by descending into the lower functions.

Functions should do one thing

When you truly have small functions, you’ll find they do one thing, and one thing only.

How do you get such small functions you may ask? Well, you start with larger functions and extract small functions from them until you can’t extract anything else meaningful. Doing this again and again will eventually leave you with functions that are sufficiently simple.

To do this, you have to be relentless in your extraction and end up with functions that in general have:

1. Between four and ten lines
2. No more than two levels of indentation

This aggressive line requirement along with few indents means that ideally, you won’t need any braces at all because the block will have been extracted to a single line method call. At most, blocks for if/else/for/while should be only a few lines long. A good rule of thumb is to extract until just before any further extraction would result in a function whose name is merely a re-stating of the implementation.

Here’s an example of one of these functions. You can clearly see the rule for how a cell survives: it has two or three live neighbors.

    private Collection getSurvivors() {
        Collection survivors = Lists.newArrayList();
        for (Cell liveCell : liveCells) {
            int liveNeighbors = countLiveNeighbors(liveCell);
            if (liveNeighbors == 2 || liveNeighbors == 3)
                survivors.add(liveCell);
        }
        return survivors;
    }

Use as few arguments as possible

Once you have small methods, you should turn your attention to the arguments of a function. It’s no surprise that we want to drive our functions towards simplicity. Arguments cause confusion; each one makes us consider how the function uses the argument and what its effects are. This means being very careful how many arguments we allow a function to have. The ideal is none.

Consider three options for this method:

1. void addSurvivors()
2. void addSurvivors(Set<Cell> nextGeneration)
3. Collection<Cell> getSurvivors()

The first option has no arguments or return value and presumably relies on an instance method to accept the added cells. This function has side effects and while it’s fairly evident what it’s doing, it’s not as explicit as it could be.

The second option is a command method, taking in a reference to the next generation and adding the surviving cells to it. This is a suitable option, but we prefer the simplicity of no arguments.

The third option is the one we’ve chosen. It’s a simple query function returning a set of surviving cells. This option has no arguments and allows the addition of survivors to the next generation to be explicitly done by the caller.

Avoid Boolean and Null arguments

There are also certain types of arguments we should look to avoid entirely: booleans and nulls.

In the case of a boolean, it’s most often a flag of some sort. When you have that coming in to your function, it is by definition doing more than one thing. Immediately the function signature indicates that it does one thing when true and another when false. And when you see only the call to the function, it’s not very clear what the flag is for. You really do have to check the function definition and hope that the coder has given you a good name as an indicator.

Nulls are something you should avoid using in your codebase where possible. It’s not always obvious what a null value means.

Check for nulls at public API boundaries, but don’t expect your methods to accept them otherwise. Accepting nulls in your methods encourages others to pass them into the function. Nulls have all the same drawbacks as booleans with the added bonus of blowing up your program.

If you really must use null and you intend to use it to mean the absence of something, have a look at Java 8’s Optional class.

Keep things organized!

We all need to exercise good code hygiene and keeping the codebase organized is a large part of that.

Organized code has functions small enough to do one thing only. These highly cohesive units of work should be named carefully and their arguments closely considered. When we take the time to do this it pays off.

Well crafted functions help us understand and maintain our codebase, but as we’ll see next week, they have another benefit still. They make our codebase easier to test. Check back in next week to find out how!

Read the Other Parts of the Clean Code Series:

• Part 1: The Difference Between Programmers and Software Engineers
• Part 2: Why Clean Code is Important for Developers and Management
• Part 3: 4 Clean Code Naming Principles Every Developer Needs to Know
• Part 4: What Every Software Engineer Ought to Know about Functions
• Part 5: Does Testing Really Make You Go Faster?
• Part 6: Teamwork Makes Clean Code Work