Java Syntax Pickiness

While fixing bugs in some Java code, I've recently come across two things that I consider poor programming syntax. Both are addressed by well known refactorings. The first one is basic programming style. The second is new and addresses a common Java trick (or hack) that is not needed once you move to Java 5 and beyond. It's an example of people not doing things the new way just because they are so use to the old way and like to copy-n-paste things.

Fail-Fast If Statements

Fail-fast is the idea that you you check error or return conditions first, get it out of the way, and then continue with the main thrust of the method. If done correctly, it makes the code more readable and avoids unnecessary indentation. Here's some simplified examples of what I've been running into.

    if (x != null && SOME_CONSTANT.equals(x)) {
        // ... some code here ...
        if (y != null && y.isSomePropertyTrue()) {
            // ... some code here ...
            if (z != null) {
                // ... some code here ...
                return true;
            } else {
                // ... some code here ...
                return false;
            }
        } else {
            return false;
        }
    } else {
        return false;
    }

    return false;

When a method that contains this sort of code is big, you often have to scroll down to see what the else block is doing and thus fully understand the method. It's better if you could understand it as you read it from top to bottom. The simplest way to refactor it, is to take the fail-fast approach and handle the various if conditions quickly and individually.

    if (x == null || ! SOME_CONSTANT.equals(x)) {
        return false;
    }

    if (y == null || ! y.isSomePropertyTrue()) {
        return false;
    }

    if (z != null) {
        // ... some code here ...
        return true;
    }

    // ... some code here ...
    return false;

Static Imports

Before static imports, a lot of folks would create an interface filled with constants. When they needed to use a constant they would implement that interface and thus be able to directly refer to the constants and not scope them to a particular class. For instance:

    String username = properties.getProperty(USER_NAME_PROPERTY);

instead of

    MyConstants.USER_NAME_PROPERTY
    String username = properties.getProperty(MyConstants.USER_NAME_PROPERTY);

It's a minor difference in syntax, but it is more readable. The problem with this approach comes into play when you generate javadocs. By default you will see all of the constants you inherited from the interface in your local class' javadoc page. If you have project-wide constant file with lots of constants, you'll end up seeing these in almost every javadoc page.

Besides this javadoc issue, my real complaint with this is that it is no longer needed. You should use static imports for constants now and stopped this old interface approach.

    import static com.foo.Bar.USER_NAME_PROPERTY;

    ...

        String username = properties.getProperty(USER_NAME_PROPERTY);
    ...

If you have a lot of constants you are using can import them all with a wildcard in the static import.

    import static com.foo.Bar.*;