Radu Cotescu's professional blog

g33k w17h pa45510n

A Useful Rant on How People Write Java Code

During the last months I’ve had the chance to see a lot of code written by different types of developers. Some of it has definitely made me proud of the word “engineering” being associated with my profession; the other, not. There are a few basic unwritten rules that more or less have become software engineering common sense.

Input handling

99.9% of the code written on this planet deals in some way or the other with user input. The User is not necessary a good person. A paranoid engineer (all engineers should be paranoid; if you’re an engineer but don’t feel paranoid just quit your job!) would actually say that The User is a terrible person whose only role is to create problems that the engineer has to solve. Since The User is such a terrible person nothing coming from them should be trusted. This is why everything that doesn’t come from a safe source (are there any safe sources for data input?!) must be validated.

For example:

1
2
3
4
5
6
7
public String reverse(String input) {
    final StringBuilder sb = new StringBuilder();
    for (int i = input.length() - 1; i >= 0; i--) {
        sb.append(input.charAt(i));
    }
    return sb.toString();
}

What would happen on line 3 if The User submitted null for the input string? That’s right. The assumption that your user is a nice person just earned you a NullPointerException. Now assume that your unvalidated code was delivered as part of some bigger project which accidentally calls your wonderful method. Oh yeah, the NPE now hogs in their code…

This could have easily been avoided by:

1
2
3
4
5
6
7
8
9
10
11
public String reverse(String input) {
    String result = null;
    if (input != null) {
        final StringBuilder sb = new StringBuilder();
        for (int i = input.length() - 1; i >= 0; i--) {
            sb.append(input.charAt(i));
        }
        result = sb.toString();
    }
    return result;
}

However, if the logic in your method is not as simple as that, always inform the caller of the consequences of calling your code with invalid parameters - whether you explicitly throw an Exception (which is documented), or you just provide a default return value.

Useful logging

Every engineer should use a logging mechanism through which meaningful and important information is… Well, logged. Probably the most useful information is the one which is logged in the case of an error. Assuming your logger supports a Throwable object as a parameter in its error method, never ever do this:

1
2
3
4
5
try {
    // code that throws a random exception
} catch (Exception e) {
    log.error("Bla bla bla" + e.getMessage());
}

It’s not gonna help anyone. Moreover it’s going to annoy the person who encounters the error because they won’t know the exact cause. Always log your errors like:

1
log.error("Meaningful error message", e);

Java is not C++ C89/ANSI C

Seriously! Repeat after me: “Java is not C++ C89/ANSI C!” You don’t need to declare your loop variable outside of the loop, like you used to do back in the old C. Nor do you have to declare and initialise a variable at the beginning of a method and then change its value on the next line. What’s the point?!

1
2
3
4
5
6
7
8
9
10
11
12
13
// int i = 0; BAD!
for (int i = 0; i < 10; i++) { /* GOOD! */
    //do something here
}

// hilarious
int index = -1;
index = "random_not_null_string".indexOf("!");
if (index == -1) {

} else {

}

Learn how to correctly concatenate Strings

Strings are immutable! Stop concatenating more than two of them with + if they are not constants. Every time you use + for something like

1
2
// assume we're in a loop
result += a + " bla bla " + b + " bla bla";

a kitten dies in terrible pain. Not only that your code is slow, it’s also occupying more memory than it should. Use one of the convenient classes for building strings, like StringBuilder or StringBuffer, the latter one being thread safe.

In case you need to write JSON

Don’t! Seriously. Just don’t. And not because I don’t like JSON, but because you’re probably going to do something wrong. Always use a class that can write JSON for you. Yes, it’s another dependency to your project. However the chances are that it’s been tested by more developers than just one. Code is one of those things where “the more, the merrier” expression just fits in without sounding awkward. Whether you use Json.simple or Gson, it’s up to you.

As a conclusion

Don’t be ashamed to ask for a code review from one of your colleagues. Nobody’s perfect, but the code you deliver should work no matter what data it’s fed through it. Write your application as if you’re building a tank. Make it handle abuse and misuse. And don’t forget to be paranoid. It helps.

(Many thanks to fellow Redditers who provided me with valuable feedback for two things that I haven’t double checked: the String concatenation example didn’t specify that + should be avoided in loops, although that was the intended case; the C++ standard allows variable declarations anywhere.)

Code, Java

« Tools required for opening a MacBook Pro Coloured log outputs on Unix/Linux »

Comments