Method Length and Extract 'Til You Drop

As a typical nerd (and a DZone Zone Leader), I tend to read a lot of blog posts and comments about programming. One thing that keeps amazing me is how long people’s functions tend to be and how many people are convinced this is the right way to code. In reality, a readable function should rarely exceed five lines of code.

Take a short look at this function:

public void show() {
    Week week = visitCalendar.getCurrentWeek();
    System.out.println(week.getStart() + " - " + week.getEnd());

    for (DayOfWeek day : VisitCalendar.OPEN_DAYS) {
        System.out.println(day + ":");
        List<Visit> visitsOnDay = visitCalendar.visitsOn(day);
        if (visitsOnDay.isEmpty()) {
            System.out.println("No visits!");
        } else {
            for (Visit visit : visitsOnDay) {
                System.out.println(visit.getTime() + ": " + visit.getOwnerName());
                System.out.println("Pets: " + visit.getPetNames());
            }
        }
    }

    try (Scanner scanner = new Scanner(System.in)) {
        String command;
        do {
            command = scanner.nextLine();
        } while (!controller.execute(command));
    }
}

How readable is that piece of code? Are you able to tell what it does just by taking a quick look at it? Two little extractions should make things clearer:

public void show() {
    Week week = visitCalendar.getCurrentWeek();
    System.out.println(week.getStart() + " - " + week.getEnd());

    for (DayOfWeek day : VisitCalendar.OPEN_DAYS) {
        System.out.println(day + ":");
        show(visitCalendar.visitsOn(day));
    }

    askForCommand();
}

private void show(List<Visit> visitsOnDay) {
    if (visitsOnDay.isEmpty()) {
        System.out.println("No visits!");
    } else {
        for (Visit visit : visitsOnDay) {
            System.out.println(visit.getTime() + ": " + visit.getOwnerName());
            System.out.println("Pets: " + visit.getPetNames());
        }
    }
}

private void askForCommand() {
    try (Scanner scanner = new Scanner(System.in)) {
        String command;
        do {
            command = scanner.nextLine();
        } while (!controller.execute(command));
    }
}

Much better. Now, the three functions are fairly readable and it should be easier to reason about what’s happening in there. But that’s still not good enough. Our show() method has mixed levels of abstraction and there’s still a loop inside an if statement. To deal with the first problem, we can simply extract two methods:

public void show() {
    show(visitCalendar.getCurrentWeek());

    for (DayOfWeek day : VisitCalendar.OPEN_DAYS) {
        show(day);
        show(visitCalendar.visitsOn(day));
    }

    askForCommand();
}

private void show(Week week) {
    System.out.println(week.getStart() + " - " + week.getEnd());
}

private void show(DayOfWeek day) {
    System.out.println(day + ":");
}

Now, how readable is that? It’s trivial, but that’s a good thing. We want our code to be trivial. Now to the second problem. Guess what we’ll do. That’s right, extract again!

private void show(List<Visit> visitsOnDay) {
    if (visitsOnDay.isEmpty()) {
        showNoVisits();
    } else {
        visitsOnDay.forEach(this::show);
    }
}

private void showNoVisits() {
    System.out.println("No visits!");
}

private void show(Visit visit) {
    System.out.println(visit.getTime() + ": " + visit.getOwnerName());
    System.out.println("Pets: " + visit.getPetNames());
}

Do you see how much clearer the code becomes? That’s exactly the point. When you extract, things become more digestible and it’s easier to preserve proper levels of abstraction. The fun part of it? You can extract all the time! Extract till you drop!

And in the meantime, your functions will become smaller and smaller. Usually less than six lines of code long. This is approximately the size at which code becomes pleasantly trivial. Therefore, beware of functions longer than that and continuously seek possibilities to extract further. The readers of your code will thank you.