Too many functions in Java code

Many java developers follow the principles of clean code. But can it be too much? I have recently reviewed code that had almost a dozen of wrappers around its private members. When is code readable enough?

When I look at the code that is so “clean” that you can’t find anything, I think of the taiga – a forest in Siberia with many small thin trees where people always get lost. It is impossible to find a way out on your own – there is just too many trees that look the same.

Letting programmers decide how many wrappers to wrap their code in is a good and healthy practice. Although do you really need to access one parameter through 10 different methods? Consider an example. You have a Worker class that takes inn some Device and checks it before doing anything:

public class Worker {

    private Device device;

    public Worker() {
        this.device = new Device();
    }

    public void doStuff() {
        if (somethingWrongWithDevice()) {
            return;
        }

        reallyDoStuff();
    }

    private boolean somethingWrongWithDevice() {
        return deviceIsNotRunning() || deviceIsNotValid();
    }

    private boolean deviceIsNotRunning() {
        return !isDeviceRunning();
    }

    private boolean deviceIsNotValid() {
        return !isDeviceValid(device);
    }

    private boolean isDeviceRunning() {
        return isDevice(device) && isDeviceEstablished(device);
    }

    private boolean isDevice(Device device) {
        return device != null;
    }

    private boolean isDeviceEstablished(Device device) {
        return device.getDeviceId() != null;
    }

    private boolean isDeviceValid(Device device) {
        return device.getDeviceId() > 0;
    }
}

Although masterminds of Java development like Clean Code author Robert Martin, Kent Back and many many others extract everything into many small methods, there are other programmers who just get a feeling of it being a bad practice. Check out this Stackexchange question: Is there such a thing as having too many private functions/methods?

Let us take this example an expand it according to the principles of Clean code:

Dirty code like this:

public static int Main()
{
    // Displays the menu.
    Console.WriteLine("Pick your option");
    Console.Writeline("[1] Input and display a polynomial");
    Console.WriteLine("[2] Add two polynomials");
    Console.WriteLine("[3] Subtract two polynomials");
    Console.WriteLine("[4] Differentiate two polynomials");
    Console.WriteLine("[0] Quit");
}

Becomes super clean:

public enum DisplayOptions {
        QUIT(0, "Quit"),
        INPUT_DISPLAY_POLYNOMIAL(1, "Input and display a polynomial"),
        ADD_TWO_POLYNOMIALS(2, "Add two polynomials"),
        SUBTRACT_TWO_POLYNOMIALS(3, "Subtract two polynomials"),
        DIFFERENTIATE_TWO_POLYNOMIALS(4, "Differentiate two polynomials");

        private final int code;
        private final String description;

        DisplayOptions(int code, String description) {
            this.code = code;
            this.description = description;
        }

        public String getDescription() {
            return description;
        }

        public int getCode() {
            return code;
        }

        @Override
        public String toString() {
            return code + ": " + description;
        }

    }

    public static int Main()
    {
        DisplayMenu();
    }

    private static void DisplayMenu()
    {
        pickYourOption();
        chooseOptions();
    }

    private static void chooseOptions() {
        displayQuit();
        displayPolynomialOptions();
    }

    private static void displayPolynomialOptions() {
        inputAndDisplayPolynomial();
        displayMathematicalOptions();
    }

    private static void displayMathematicalOptions() {
        addTwoPolynomials();
        subtractTwoPolynomials();
        DifferentiateTwoPolynomials();
    }

    private static void displayQuit() {
        DisplayOptions quitOption = getDisplayOptionQuit();
        buildTextAndWriteToConsole(quitOption);
    }

    private static void DifferentiateTwoPolynomials() {
        DisplayOptions quitOption = getDisplayOptionDifferentiateTwoPolynomials();
        buildTextAndWriteToConsole(quitOption);
    }

    private static void subtractTwoPolynomials() {
        DisplayOptions quitOption = getDisplayOptionSubtractTwoPolynomials();
        buildTextAndWriteToConsole(quitOption);
    }

    private static void addTwoPolynomials() {
        DisplayOptions quitOption = getDisplayOptionAddTwoPolynomials();
        buildTextAndWriteToConsole(quitOption);
    }

    private static void inputAndDisplayPolynomial() {
        DisplayOptions quitOption = getDisplayOptionInputAndDisplayPolynomial();
        buildTextAndWriteToConsole(quitOption);
    }

    private static void pickYourOption() {
        writeToConsole("Pick your option");
    }

    private static void buildTextAndWriteToConsole(DisplayOptions quitOption) {
        String text = buildDisplayText(quitOption.getCode(), quitOption.getDescription());
        writeToConsole(text);
    }

    private static void writeToConsole(String text) {
        Console.WriteLine(text);
    }

    private static DisplayOptions getDisplayOptionQuit() {
        return DisplayOptions.QUIT;
    }

    private static DisplayOptions getDisplayOptionDifferentiateTwoPolynomials() {
        return DisplayOptions.DIFFERENTIATE_TWO_POLYNOMIALS;
    }

    private static DisplayOptions getDisplayOptionSubtractTwoPolynomials() {
        return DisplayOptions.SUBTRACT_TWO_POLYNOMIALS;
    }

    private static DisplayOptions getDisplayOptionAddTwoPolynomials() {
        return DisplayOptions.ADD_TWO_POLYNOMIALS;
    }

    private static DisplayOptions getDisplayOptionInputAndDisplayPolynomial() {
        return DisplayOptions.INPUT_DISPLAY_POLYNOMIAL;
    }

    private static String buildDisplayText(int number, String text) {
        StringBuilder sb = new StringBuilder();
        sb.append("[ ");
        sb.append(number);
        sb.append(" ] ");
        sb.append(text);
        return sb.toString();
    }

You can make it even cleaner! Extract all this into several classes, for example DisplayPolynomial that derives from parent classes Display or Polynomial or both. Add wrappers OptionsMenu and DisplayMenu. Add interfaces so no one could call your function directly. Hide everything from everybody – it is called Encapsulation and Abstraction Control!

Also, do not forget to test EACH function! Because 100% “test coverage” is very important!

The result is a readable code, yes. But is it easy to grasp the meaning of different code chunks? When you expand 1 method with 5 lines into a pack of 5-10 classes with 100 methods each – you will get hard time explaining logic to new programmers. They will be able to read, but they won’t be able to understand the flow of the program. This will result in a huge program with a lot of redundant classes that will require a lot of resources to maintain and make changes. New people will think it’s easier to write new models, than to find a place to inject new logic. The code becomes huge forest of small thin classes and methods – like the taiga in Siberia.

Further I will try and combine ideas and experiences of different people, including Uncle Bob and usual programmers.

Simple

Functions should be small, but not too small. The logic should be visible right there without programmers needing to spend several hours undressing wrappers like Russian matroshka.

Do not write small redundant functions that simply return one value:

private static DisplayOptions getDisplayOptionQuit() {
    return DisplayOptions.QUIT;
}

I would also argue to not extract functions that check if the value is null:

private boolean isWorkerAvailable() {
    return worker != null;
}

That is just too little logic going on. It makes it difficult to search for errors like application hanging. Your program won’t crash, but it will be difficult to find why it doesn’t respond. You trust a function that is called isWorkerAvailable without questioning how it is implemented.

It is easier to question the logic of this function:

private void doStuff() {
    if(worker != null) {
        actuallyDoStuff();
    }
}

What happens if the worker is NULL? Should we notify the user about it?

In the contrary to this function:

private void doStuff() {
    if(isWorkerAvailable()) {
        actuallyDoStuff();
    }
}

This code gives sign of it being sure that the implementation is clean. We won’t touch or question it as easily.

Rule of thumb:
– an average function should be 5-15 lines long. If you have many functions with less than 5 lines it can be a sign that you are breaking your code too much.
– a function should show all conceptual logic together so that it fits on your screen and doesn’t require scrolling.

Do One thing

A function should do one logical thing, it should do it good and it should do it only. Do not mix “logical thing” with mathematical operations. Simple null check is not the reason for its own function. Wrapper around other functions is not a good reason either.

You can wrap anything in many different layers of functions. Ask yourself if you really need a function here or are these functions tell you everything by themselves.

Do not do this:

private void checkOk(Thing thing) {
        if(exists(thing)) {
            return isThingOk(thing);
        }
    }

    private boolean isThingOk(Thing thing) {
        return isTypeOk(thing) && isFormOk(thing);
    }

    private boolean isFormOk(Thing thing) {
        return thing.form == Form.OK;
    }

    private boolean isTypeOk(Thing thing) {
        return thing.type == Type.OK;
    }

    private boolean exists(Thing thing) {
        return thing != null;
    }

What is wrong with the following simple function?

private void checkOk(Thing thing) {
    return thing != null 
        && thing.type == Type.OK 
        && thing.form == Form.OK;
}

Rule of thumb:
– function should do one logical thing.
– do not wrap functions within functions for the sake of making functions.

Do not write Clean code just for the sake of writing it clean. Write it like a book where it is easy for other programmers to read and understand the meaning.