Skip to content
Yegor Bugayenko edited this page Sep 17, 2013 · 32 revisions

This page contains most typical mistakes I see in Java code of people working with me. Qulice can't catch them for obvious reasons, that's why I created this page. Let me know if you want to see something else here, I'll be happy to add.

Class Names

Read this short "What is an Object?" article. Your class should be an abstraction of a real life entity. No "validators", "controllers", "managers", etc. If your class name ends with an "-er" - it's a bad design.

And, of course, utility classes are anti-patterns, like that StringUtils, FileUtils, and IOUtils from Apache. They are perfect examples of a terrible design.

Method Names

Method can either return something or return void. If it returns something than its name should explain what it returns, for example (don't use get prefix ever):

boolean isValid(String name);
String content();
int ageOf(File file);

If it returns void than its name should explain what it does, for example:

void save(File file);
void process(Work work);
void append(File file, String line);

There is only one exception to the rule just explained - test methods for JUnit. They are explained below.

Test Method Names

Method names in JUnit tests should be created as English sentences without spaces. It's easier to explain by example:

/**
 * HttpRequest can return its content in Unicode.
 * @throws Exception If test fails
 */
public void returnsItsContentInUnicode() throws Exception {
}

It's important to start your JavaDoc first sentence with the name of the class you're testing followed by can. So, your first and the only sentence will always sound like "somebody can do something". Method name will say exactly the same, but without the subject. If I add a subject at the beginning of the method name I should get a complete English sentence, as in example above: "HttpRequest returns its content in unicode".

Pay attention that test method doesn't start with can. Only JavaDoc comment starts so. Method name doesn't need starts with a verb.

It's a good practice to always declare test methods as throwing Exception.

Variable Names

Avoid composite names of variables, like timeOfDay, firstItem, or httpRequest. I mean both class variables and in-method ones. A variable name should be long enough to avoid ambiguity in its scope of visibility, but not longer. A name should be a noun in singular or plural form, or its abbreviation, for example:

final List<String> names;
void sendThroughProxy(File file, Protocol proto);
private File content;
public HttpRequest request;

Sometimes you may have collisions between constructor parameters and in-class properties, when your constructor is saving incoming data into an instantiated object. In this case I recommend to use abbreviate by removing vowels (see how USPS abbreviates street names), for example:

public final class Message {
  private String recipient;
  public Message(final String rcpt) {
    this.recipient = rcpt;
  }
}

In many cases, the best hint for a name of a variable you can get by reading its class name. Just write it with a small letter and you're good:

File file;
User user;
Branch branch;

But never do the same for primitive types, like Integer number or String string.

You can also you an adjective, when there are more than one variable of different characteristics, for example:

String contact(String left, String right);

Constructors

Without exceptions, there should be only one constructor that stores data into object variables. All other constructors should call this one with different arguments. For example:

public class Server {
  private final String address;
  public Server(String uri) {
    this.address = uri;
  }
  public Server(URI uri) {
    this(uri.toString());
  }
}

One-time Variables

Avoid one-time variables at all cost. By one-time I mean variables that are used only once, for example:

final String name = "data.txt";
return new File(name);

This variable is used only once and this code should be refactored to:

return new File("data.txt");

Sometime, in very rare cases, mostly because of better formatting, one-time variables may be used. But try to avoid such situations at all cost.

Exceptions

Needless to say that you should never swallow exceptions, but let them bubble up as high as possible.

Never use exceptions for flow control, for example this code is wrong:

int size;
try {
  size = this.fileSize();
} catch (IOException ex) {
  size = 0;
}

Seriously, what if that IOException says "disk is full"? You will still assume that the size of the file is zero and move on?

Time Constants

Don't calculate time with constants, like:

final long millis = min * 60 * 1000;

Instead, use TimeUnit:

final long millis = TimeUnit.MINUTES.toMillis(2);
final long sec = TimeUnit.HOURS.toMinutes(1);

Lombok

Don't forget to use @ToString and @EqualsAndHashCode annotations from Lombok. They add toString() and equals() and hashCode() methods to your class during compilation. Why it's important? Because without them you will either 1) have default methods that don't do any good, or 2) will hav to create your own in every class, which is ineffective.

But don't forget to specify a list of private properties for @EqualsAndHashCode, for example:

@ToString
@EqualsAndHashCode(of = { "name", "age" })
public final class User {
  private final transient String name;
  private final transient long age;
}

Indentation

Main simple rule is that a bracket should either end a line or be closed on the same line (reverse rule applies to a closing bracket). For example, this is not correct, because the first bracket is not closed on the same line and there are symbols after it. The second bracket is also in trouble, because there are symbols in front of it and it is not opened at the same line:

final File file = new File(directory,
  "file.txt");

Correct indentation should look like:

StringUtils.join(
  Arrays.asList(
    "first line",
    "second line",
    StringUtils.join(
      Arrays.asList("a", "b")
    )
  ),
  "separator"
);

The second important rule says that you should put as much as possible on one line, within the limit of 80 characters. The example above is not valid, since it can be compacted:

StringUtils.join(
  Arrays.asList(
    "first line", "second line",
    StringUtils.join(Arrays.asList("a", "b"))
  ), 
  "separator"
);
Clone this wiki locally