I am the founder and CEO of Data Geekery GmbH, located in Zurich, Switzerland. With our company, we have been selling database products and services around Java and SQL since 2013. Ever since my Master's studies at EPFL in 2006, I have been fascinated by the interaction of Java and SQL. Most of this experience I have obtained in the Swiss E-Banking field through various variants (JDBC, Hibernate, mostly with Oracle). I am happy to share this knowledge at various conferences, JUGs, in-house presentations and on our blog. Lukas is a DZone MVB and is not an employee of DZone and has posted 224 posts at DZone. You can read more from them at their website. View Full User Profile

Top Posts of 2013: 10 Subtle Best Practices when Coding Java

12.31.2013
| 74755 views |
  • submit to reddit

This is a list of 10 best practices that are more subtle than your average Josh Bloch Effective Java rule. While Josh Bloch’s list is very easy to learn and concerns everyday situations, this list here contains less common situations involving API / SPI  design that may have a big effect nontheless.

I have encountered these things while writing and maintaining jOOQ, an internal DSL modelling SQL in Java. Being an internal DSL, jOOQ challenges Java compilers and generics to the max, combining generics, varargs and overloading in a way that Josh Bloch probably wouldn’t recommend for the “average API”.

Let me share with you 10 Subtle Best Practices When Coding Java:

1. Remember C++ destructors

Remember C++ destructors? No? Then you might be lucky as you never had to debug through any code leaving memory leaks due to allocated memory not having been freed after an object was removed. Thanks Sun/Oracle for implementing garbage collection!

But nonetheless, destructors have an interesting trait to them. It often makes sense to free memory in the inverse order of allocation. Keep this in mind in Java as well, when you’re operating with destructor-like semantics:

  • When using @Before and @After JUnit annotations
  • When allocating, freeing JDBC resources
  • When calling super methods

There are various other use cases. Here’s a concrete example showing how you might implement some event listener SPI:

@Override
public void beforeEvent(EventContext e) {
  super.beforeEvent(e);
  // Super code before my code
}

@Override
public void afterEvent(EventContext e) {
  // Super code after my code
  super.afterEvent(e);
}

Another good example showing why this can be important is the infamous Dining Philosophers problem. More info about the dining philosophers can be seen in this awesome post:

http://adit.io/posts/2013-05-11-The-Dining-Philosophers-Problem-With-Ron-Swanson.html 

The Rule: Whenever you implement logic using before/after, allocate/free, take/return semantics, think about whether the after/free/return operation should perform stuff in the inverse order.

2. Don’t trust your early SPI evolution judgement

Providing an SPI to your consumers is an easy way to allow them to inject custom behaviour into your library / code. Beware, though, that your SPI evolution judgement may trick you into thinking that you’re (not) going to need that additional parameter. True, no functionality should be added early. But once you’ve published your SPI and once you’ve decided followingsemantic versioning, you’ll really regret having added a silly, one-argument method to your SPI when you realise that you might need another argument in some cases:

interface EventListener {
  // Bad
  void message(String message);
}

What if you also need a message ID and a message source? API evolution will prevent you from adding that parameter easily, to the above type. Granted, with Java 8, you could add a defender method, to “defend” you bad early design decision:

interface EventListener {
  // Bad
  default void message(String message) {
    message(message, null, null);
  }
  // Better?
  void message(
    String message,
    Integer id,
    MessageSource source
  );
}

Note, unfortunately, the defender method cannot be made final.

But much better than polluting your SPI with dozens of methods, use acontext object (or argument object) just for this purpose.

interface MessageContext {
  String message();
  Integer id();
  MessageSource source();
}

interface EventListener {
  // Awesome!
  void message(MessageContext context);
}

You can evolve the MessageContext API much more easily than the EventListener SPI as fewer users will have implemented it.

The Rule: Whenever you specify an SPI, consider using context / parameter objects instead of writing methods with a fixed amount of parameters.

Remark: It is often a good idea to also communicate results through a dedicated MessageResult type, which can be constructed through a builder API. This will add even more SPI evolution flexibility to your SPI.

3. Avoid returning anonymous, local, or inner classes

Swing programmers probably have a couple of keyboard shortcuts to generate the code for their hundreds of anonymous classes. In many cases, creating them is nice as you can locally adhere to an interface, without going through the “hassle” of thinking about a full SPI subtype lifecycle.

But you should not use anonymous, local, or inner classes too often for a simple reason: They keep a reference to the outer instance. And they will drag that outer instance to wherevery they go, e.g. to some scope outside of your local class if you’re not careful. This can be a major source for memory leaks, as your whole object graph will suddenly entangle in subtle ways.

The Rule: Whenever you write an anonymous, local or inner class, check if you can make it static or even a regular top-level class. Avoid returning anonymous, local or inner class instances from methods to the outside scope.

Remark: There has been some clever practice around double-curly braces for simple object instantiation:

new HashMap<String, String>() {{
  put("1", "a");
  put("2", "b");
}}

This leverages Java’s instance initializer as specified by the JLS §8.6. Looks nice (maybe a bit weird), but is really a bad idea. What would otherwise be a completely independent HashMap instance now keeps a reference to the outer instance, whatever that just happens to be. Besides, you’ll create an additional class for the class loader to manage.

4. Start writing SAMs now!

Java 8 is knocking on the door. And with Java 8 come lambdas, whether you like them or not. Your API consumers may like them, though, and you better make sure that they can make use of them as often as possible. Hence, unless your API accepts simple “scalar” types such as intlongStringDate, let your API accept SAMs as often as possible.

What’s a SAM? A SAM is a Single Abstract Method [Type]. Also known as afunctional interface, soon to be annotated with the @FunctionalInterface annotation. This goes well with rule number 2, where EventListener is in fact a SAM. The best SAMs are those with single arguments, as they will further simplify writing of a lambda. Imagine writing

listeners.add(c -> System.out.println(c.message()));

Instead of

listeners.add(new EventListener() {
  @Override
  public void message(MessageContext c) {
    System.out.println(c.message()));
  }
});

Imagine XML processing through jOOX, which features a couple of SAMs:

$(document)
  // Find elements with an ID
  .find(c -> $(c).id() != null)
  // Find their child elements
  .children(c -> $(c).tag().equals("order"))
  // Print all matches
  .each(c -> System.out.println($(c)))

The Rule: Be nice with your API consumers and write SAMs / Functional interfaces already now.

Remarks: A couple of interesting blog posts about Java 8 Lambdas and improved Collections API can be seen here:

5. Avoid returning null from API methods

I’ve blogged about Java’s NULLs once or twice. I’ve also blogged about Java 8′s introduction of Optional. These are interesting topics both from an academic and from a practical point of view.

While NULLs and NullPointerExceptions will probably stay a major pain in Java for a while, you can still design your API in a way that users will not run into any issues. Try to avoid returning null from API methods whenever possible. Your API consumers should be able to chain methods whenever applicable:

initialise(someArgument).calculate(data).dispatch();

In the above snippet, none of the methods should ever return null. In fact, using null’s semantics (the absence of a value) should be rather exceptional in general. In libraries like jQuery (or jOOX, a Java port thereof), nulls are completely avoided as you’re always operating on iterable objects. Whether you match something or not is irrelevant to the next method call.

Nulls often arise also because of lazy initialisation. In many cases, lazy initialisation can be avoided too, without any significant performance impact. In fact, lazy initialisation should be used carefully, only. If large data structures are involved.

The Rule: Avoid returning nulls from methods whenever possible. Use null only for the “uninitialised” or “absent” semantics.

6. Never return null arrays or lists from API methods

While there are some cases when returning nulls from methods is OK, there is absolutely no use case of returning null arrays or null collections! Let’s consider the hideous java.io.File.list() method. It returns:

An array of strings naming the files and directories in the directory denoted by this abstract pathname. The array will be empty if the directory is empty. Returns null if this abstract pathname does not denote a directory, or if an I/O error occurs.

Hence, the correct way to deal with this method is

File directory = // ...

if (directory.isDirectory()) {
  String[] list = directory.list();

  if (list != null) {
    for (String file : list) {
      // ...
    }
  }
}

Was that null check really necessary? Most I/O operations produce IOExceptions, but this one returns null. Null cannot hold any error message indicating why the I/O error occurred. So this is wrong in three ways:

  • Null does not help in finding the error
  • Null does not allow to distinguish I/O errors from the File instance not being a directory
  • Everyone will keep forgetting about null, here

In collection contexts, the notion of “absence” is best implemented by empty arrays or collections. Having an “absent” array or collection is hardly ever useful, except again, for lazy initialisation.

The Rule: Arrays or Collections should never be null.

7. Avoid state, be functional

What’s nice about HTTP is the fact that it is stateless. All relevant state is transferred in each request and in each response. This is essential to the naming of REST: Representational State Transfer. This is awesome when done in Java as well. Think of it in terms of rule number 2 when methods receive stateful parameter objects. Things can be so much simpler if state is transferred in such objects, rather than manipulated from the outside. Take JDBC, for instance. The following example fetches a cursor from a stored procedure:

CallableStatement s =
  connection.prepareCall("{ ? = ... }");

// Verbose manipulation of statement state:
s.registerOutParameter(1, cursor);
s.setString(2, "abc");
s.execute();
ResultSet rs = s.getObject(1);

// Verbose manipulation of result set state:
rs.next();
rs.next();

These are the things that make JDBC such an awkward API to deal with. Each object is incredibly stateful and hard to manipulate. Concretely, there are two major issues:

  • It is very hard to correctly deal with stateful APIs in multi-threaded environments
  • It is very hard to make stateful resources globally available, as the state is not documented
State is like a box of chocolates

Theatrical poster for Forrest Gump, Copyright © 1994 by Paramount Pictures. All Rights Reserved. It is believed that the above usage fulfils what is known as Fair Use

The Rule: Implement more of a functional style. Pass state through method arguments. Manipulate less object state.

8. Short-circuit equals()

This is a low-hanging fruit. In large object graphs, you can gain significantly in terms of performance, if all your objects’ equals() methods dirt-cheaply compare for identity first:

@Override
public boolean equals(Object other) {
  if (this == other) return true;
  // Rest of equality logic...
}

Note, other short-circuit checks may involve null checks, which should be there as well:

@Override
public boolean equals(Object other) {
  if (this == other) return true;
  if (other == null) return false;
  // Rest of equality logic...
}

The Rule: Short-circuit all your equals() methods to gain performance.

9. Try to make methods final by default

Some will disagree on this, as making things final by default is quite the opposite of what Java developers are used to. But if you’re in full control of all source code, there’s absolutely nothing wrong with making methods final by default, because:

  • If you do need to override a method (do you really?), you can still remove the final keyword
  • You will never accidentally override any method anymore

This specifically applies for static methods, where “overriding” (actually, shadowing) hardly ever makes sense. I’ve come across a very bad example of shadowing static methods in Apache Tika, recently. Consider:

TikaInputStream extends TaggedInputStream and shadows its static get() method with quite a different implementation.

Unlike regular methods, static methods don’t override each other, as the call-site binds a static method invocation at compile-time. If you’re unlucky, you might just get the wrong method accidentally.

The Rule: If you’re in full control of your API, try making as many methods as possible final by default.

10. Avoid the method(T…) signature

There’s nothing wrong with the occasional “accept-all” varargs method that accepts an Object... argument:

void acceptAll(Object... all);

Writing such a method brings a little JavaScript feeling to the Java ecosystem. Of course, you probably want to restrict the actual type to something more confined in a real-world situation, e.g. String.... And because you don’t want to confine too much, you might think it is a good idea to replace Object by a generic T:

void acceptAll(T... all);

But it’s not. T can always be inferred to Object. In fact, you might as well just not use generics with the above methods. More importantly, you may think that you can overload the above method, but you cannot:

void acceptAll(T... all);
void acceptAll(String message, T... all);

This looks as though you could optionally pass a String message to the method. But what happens to this call here?

acceptAll("Message", 123, "abc");

The compiler will infer <? extends Serializable & Comparable<?>> for T, which makes the call ambiguous!

So, whenever you have an “accept-all” signature (even if it is generic), you will never again be able to typesafely overload it. API consumers may just be lucky enough to “accidentally” have the compiler chose the “right” most specific method. But they may as well be tricked into using the “accept-all” method or they may not be able to call any method at all.

The Rule: Avoid “accept-all” signatures if you can. And if you cannot, never overload such a method.

Conclusion

Java is a beast. Unlike other, fancier languages, it has evolved slowly to what it is today. And that’s probably a good thing, because already at the speed of development of Java, there are hundreds of caveats, which can only be mastered through years of experience.

Stay tuned for more top 10 lists on the subject!

Published at DZone with permission of Lukas Eder, author and DZone MVB. (source)

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Robert Saulnier replied on Wed, 2013/08/21 - 12:06pm

Here's a few comments on your top 10:

1. Funny thing. I just implemented destructor'ish functionality in Java using PhantomReferences.

3. Avoid leaking/returning anonymous, local, or inner classes.

8. If the IDE doesn't do this for already, consider changing the template.

9. Try to make classes/fields/methods final by default.

Lukas Eder replied on Thu, 2013/08/22 - 12:58am in response to: Robert Saulnier

1. "Destructors" are everywhere

3. I agree. That point was really about returning, and thus leaking. Fixed

8. True. But it's always good to think about the rationale of some IDE actions, from time to time

9. classes/fields will be mentioned in another blog post :-)

Ricardo Lima replied on Thu, 2013/08/22 - 6:42am

 What's an SPI? It's the first time I saw this term. It seems to mean Service Programming Interface. Is that it? You defined SAM (Single Abstract Method), but skipped SPI.

Lukas Eder replied on Thu, 2013/08/22 - 7:06am in response to: Ricardo Lima

Robert Saulnier replied on Thu, 2013/08/22 - 7:07am in response to: Ricardo Lima

SPI => Service Provider Interface

You'll notice that there are some packages in Java SE that end in spi, i.e. java.nio.channels.spi

Philippe Lhoste replied on Wed, 2013/09/04 - 5:55am

"You will never accidentally override any method anymore"

That's what the @Override notation is for... :) A good IDE will make an overridden method without this annotation with a warning.

In my equals methods, I don't explicitly check for null, I just use:

if (!(obj instanceof CurrentClass)) return false;

because it also covers the null case. Won't work if you prefer to use getClass()...

Overall, good article. Thanks.

Robert Saulnier replied on Wed, 2013/09/04 - 7:39am in response to: Philippe Lhoste

@Override doesn't stop you from accidentally overriding a method. The method getMeaningOfLife() must always return 42:

public class Super {
    public int getMeaningOfLife() {
        return 42;
    }
}

But if your method is not final, nothing is stopping you from overriding the method:

public class Sub extends Super {
    @Override
    public int getMeaningOfLife() {
        return -1;
    }
}

@Override is used to document that a method overrides another.

As for using instanceof in the equals method, it's ok if the class is final, but if the class is not final then subclasses can break symmetry. From Obejct.equals(Object):

It is symmetric: for any non-null reference values x and y, x.equals(y)
should return true if and only if y.equals(x) returns true.


Hans Aikema replied on Wed, 2013/09/04 - 8:46am in response to: Robert Saulnier

@Override is meant to document that a method's intention is to override a method of a superclass, not that it actually overrides a method of the superclass (which is a minor, but relevant semantic difference). The most usefull feature of the annotation is that the compiler must signal if an @Override annotated method does not override a method of a superclass.

Oracle JavaDoc for @Override

Robert Saulnier replied on Wed, 2013/09/04 - 9:01am in response to: Hans Aikema

From the @Override Javadoc:

Indicates that a method declaration is intended to override a method declaration in a superclass. If a method is annotated with this annotation type but does not override a superclass method, compilers are required to generate an error message.

If you have a method with the @Override annotation, it overrides a method or you get a compiler error.

Bob Fields replied on Wed, 2013/09/04 - 9:13am

 8. Short circuit .equals(), second code segment:

Shouldn't you check (other == null) before checking (this == other)?

Overall good article.

Lukas Eder replied on Wed, 2013/09/04 - 9:18am in response to: Bob Fields

Shouldn't you check (other == null) before checking (this == other)?

I've taken inspiration from what is generated by Eclipse when generating hashCode() and equals(). I guess (this == other) just happens way more often than (other == null). So in general, you can save a nano second per hour ;-)

Bob Fields replied on Wed, 2013/09/04 - 9:41am in response to: Lukas Eder

 OK makes sense. (this == other) doesn't generate a NullPointerException or other error when other is null, since no actual method call is made on the other object, and only returns true when 'this' object is null.

Robert Saulnier replied on Wed, 2013/09/04 - 9:45am in response to: Bob Fields

"this" will never be null. I think you meant "other"?

Hans Aikema replied on Wed, 2013/09/04 - 9:46am

6. Never return null arrays or lists from API methods

I would say that should read 6. Never return null arrays or lists from methods. No need to restrict this common-sense behaviour to API methods. You shouldn't have to use handling null for list/array-based return values for private method calls either.

Philippe Lhoste replied on Wed, 2013/09/04 - 1:57pm in response to: Robert Saulnier

What you show is not "accidental", it was my point (and your)... It is a deliberate overriding.

So you don't use final to avoid "accidental" overrides, but because you don't want them to be overridden, for some reason specific to the class.

As for using instanceof, I don't know, but Joshua Bloch recommends and shows (in Effective Java) its usage: "Use the instanceofoperator to check if the argument has the correct type"

Also of interest on the topic: http://www.artima.com/lejava/articles/equality.html

Raging Infernoz replied on Wed, 2013/09/04 - 4:07pm

Re 8:
An equals should always do this, otherwise it is breaking the equals contract.
public boolean equals(Object o)
  if (o instanceof MyClassName) {
     // do compares, return result.
  }
  return false;
}

Null checks of the external object in equals really is completely pointless if instanceof is used first.

Grab a copy of FindBugs and PMD, and turn the lint options on for Javac too, and be prepared to be shocked; I use them routinely (with some nonsense rules disabled), and they've improved my code quality and spotted some horrific clangers in inherited code (shudder!) e.g. infinite loops for polymorphic methods!  I really can't stand Eclipse, it is way to noisy and klunky for my taste, so use NetBean 7.3.1 which has it's own checks, and has the Inspect tool scan to for and give you the choice to automatically fix lots of other iffy code, including: missing @Override attributes, and unused imports.

Re 10:
This looks nonsense; just bound T properly <T extends bla> or <T super bla>, then it isn't ambiguous.

Generics are great, provided you learn how to use them properly, and don't stupidly try to over do polymorphism for a particular method name, when you really should use a different method name.  Using just Object... can actually be much worse!

I've pulled the embedded ParameterizedType out of Generic typed sub-classes (thanks Stack Overflow), via reflection, to simplify mapping code; it could also be used for other purposes.  Yes, the Generic Types can be stored during erasure of sub-classes.

Raging Infernoz replied on Wed, 2013/09/04 - 4:24pm

 Re 9:

I think you find that static methods are always tied to a specific class, so declaring static methods as final is stupid, because you can always 'redefine' static methods in a sub-class, and PMD will tell you off when you try this nonsense.


Raging Infernoz replied on Wed, 2013/09/04 - 4:28pm

I hope you get on, or are on, the "Java Specialists" and "Java Performance Tuning" mailing lists, because they have articles which blow away plenty of myths in Java, and show how it really works.


Valery Silaev replied on Fri, 2014/01/03 - 6:16pm

Cheap rule from my previous C experience -- constant should be on left in comparison!

My first language was ObjectPascal, so when starting development in C  I oftently confused =/== operators that leads to very hard-to-find errors:

if (c = 3) // Bang

Instead, the error below is always catched by compiler

if (3 = c) // Compiler error

In Java this habit helps to avoid NPE-s:

if ("some const".equals(someVar))

Btw, "constant on the left" is extremely important practice for JS developers.

Valery Silaev replied on Fri, 2014/01/03 - 6:21pm

Interesting, for rule (6) what would you suggest as empty value:

Collections.emptyList() // unmodifiable

Or

new ArrayList() // modifiable

?

Lukas Eder replied on Sat, 2014/01/04 - 1:48am in response to: Valery Silaev

The constant on the left rule is quite interesting! In Java, it's not applicable, because only boolean expressions can be put in if statements, and no one would make a boolean comparison like this:

if (true == a) { ... }
You would simply write:
if (a) { ... }
But a good rule to generally keep in mind, e.g. for less typesafe languages as C and JavaScript!


For rule (6), well, if mutability isn't important, return the cheap emptyList() from a method (I suppose this is about method results?). It may just be a micro-optimisation in most cases. But if the method is called often, then you may slightly decrease the burden on the GC

Valery Silaev replied on Sat, 2014/01/04 - 5:48am in response to: Lukas Eder

Lukas,

I mean that in Java this is similar how to apply "equals" method call -- whenever possible put constant on the left, i. e. const.equals(variable) - this reduces chances for NullPointerException while const is not null, but variable may be null.

Mutable vs immutable collections as return values may be a pain for API users. Personally, I follow this rule: if your code return mutable non- empty collection, than never return immutable zero-size collection for consistency. 

Chanchal Goyal replied on Mon, 2014/06/23 - 1:39pm in response to: Philippe Lhoste

I think if you use the instanceof test in the equals method, as a rule of thumb the equals method should also be final.


Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.