Methods should not return or handle null, except if they indicate an optional operation
- For optional Operations (like Map.get(key)), it must be clearly documented. This should be used seldomly, since it cannot be checked by the Compiler and the programmer must remember to ALWAYS handle null.
- Methods which return collections or Strings should return Collections.emptyList / "" rather than null, even for optional values.
- Never return null if an error occurs. Throw a checked exception if the error may happen and an unchecked Exception (usually, IllegalStateException) if the error must not happen and the app cannot recover from it.
- Set all fields of a bean with initial values in the constructor to prevent them from being null.
- Null input parameters should result immediatly in an IllegalArgumentException, to prevent these errors from waiting under the surface.
- Default Values: Sometimes, it is possible to define a natural empty or default value for some classes. In these cases, the empty element can be a static final constant (if it is immutable) attached to the class and should be used in all cases were normally "null" would be used. Use this whereever appropiate - its way better than using null. Default Values can also be used as fallbacks when throwing an exception would not be possible, but some kind of value must be given as a certain point if everything else fails...
Since JDK-Collections are always mutable by their interface, some additional contracts must be defined:
- Methods should by all means never modify input parameter collections. It can be done on private methods, but it shouldn't be on public ones. Instead of changing the input collection, a new one should be created and returned. The performance inpact is negibligible, but the method becomes easier to maintain and understand.
- Public methods returning collections must always return unmodifiable Collections, usually by wrapping the result with Collections.unmodifiableList (...). This makes the method more flexible, as it can return constants like Collections.emptyList(). If a method returns a modifiable list, the result would be sometimes mutable and sometimes not (the EmptyList constant) - thats just asking for trouble.
- Beans which have collection properties MUST always wrap the results of their public getters in Collections.unmodifiableList, if the list is mutable. Collection properties may only be changed by setters - which replace the collection with another one - and explicit public mutators like public void add(...) on the bean itself. Changing collection properties in-place has countless side-effects and is very hard to maintain.
- Whenever a collection of exactly one element should be created, then it should be done via Collections.singleton(Object).
- If mutable collections are used as static fields or in singletons (most common static field: a (caching) Map), then the field must be synchronized via Collections.synchronized(...). or the access must be synchronized by hand. The later is the better choice, as it allows a more fine-grained control over the synchronization blocks. In general, just try to avoid mutable static collections, if you do not need them for performance reasons.
- Try to use google collections. Google Collections have the ImmutableList/-Set/-Map, which allow it to create immutable collections without wrappers - yielding nice performance benefits on the way. These are so important that its really a pity that the JDK does not support these directly.
If it is possible that your program runs into an unstable / unexpected state, then you must throw a runtime exception indicating that there's a programmer's error in there.
- Throw an IllegalArgumentException if your input is bad, esp. for null input values. Write static convenience methods for null checks, and use them in all setters and constructors where a null-check is needed.
- Throw an IllegalStateException if your program is in an invalid state (bad assigned fields, wrong execution order..). This usually replaces returning null as an idiom if something fails.
- Throw an UnsupportedOperationException if you cannot support a method derived from an interface, like write operations on immutable objects.
- Throw an IndexOutOfBoundsException for crappy index accesses.
- Throw an ClassCastExceptoin (or let the jvm do that for you in an unsafe cast) if your have the wrong object in your hands.
Of course there are more, but these are the most common... In general, all of them except the IllegalArgumentException indicate a (minor) error in your design: A good design tries to enforce compile-time-safety rather than handling problems later with runtime-exceptions:
- IllegalStateExceptions can be avoided by using immutable objects as much as possible (see below), as they do not have states.
- UnsupportedOperationExceptions can be prevented by defining "ReadInterfaces" and "WriteInterfaces" seperately. In general, the WriteInterfaces then inherit from the "ReadInterfaces" - also, try in general to define "thin" interfaces rather than rich interfaces. the JDK collections-library is a perfect antipattern for this.
- IndexOutOfBoundsException: Index-Based-Accesses are in general a problem of the design. You should never rely on positions in your arrays, lists or strings. Instead, create your own classes which handle these problems. Also, you should never iterate within a "for"-loop on an index. for Lists: Use the iterator ( JDK 1.5: java's foreach) instead.
- ClassCastExceptions, instanceof checks and casts are the bastard-stepchilds of the pre-JDK1.5 era. Starting from JDK 1.5, nearly all these usecases can be cleanly rewritten by using compile-time-checked generics. Also, you should never ever use collections with different kinds of objects in them!
At its best, a program relies solely on IllegalArgumentExceptions for null-checking and the like to fortify its code base and the maintainability of the code.
Do not swallow checked exceptions - re-throw them unchecked instead!
Everybody tells java programmers that they must not swallow exceptions. Good.
But: Nearly no one tells them what to do instead.
Its no wonder that a lot of programmers rather write empty catch blocks (perhaps with a log statement, if they where i a nice mood) for checked exceptions which they think can never happen.. Unless the program is really screwed or there is an error in the logic.
But: Nearly no one tells them what to do instead.
Its no wonder that a lot of programmers rather write empty catch blocks (perhaps with a log statement, if they where i a nice mood) for checked exceptions which they think can never happen.. Unless the program is really screwed or there is an error in the logic.
The solution, however, is very simple. Wrap them in a runtime exception, like this:
try{
Class.forName("horst");
}catch(ClassNotFoundException e){
// have to die if this would ever happen, but need to catch it
throw new IllegalStateException(e);
// If you have the apache-commons-lang.jar available, you should prefer using this:
// If you have the apache-commons-lang.jar available, you should prefer using this:
// throw new UnhandledException(e);
}
Nice, hu? That way, you tell the world that this is an unrecoverable condition and by sure a programming error.
However, whenever you do this, you however have to remind the following: Never, ever do this on a condition which may fail, and does not just indicate a programming error if it is thrown:
If the programmer has made a mistake, a RuntimeException and an app crash are appropiate.
If however a File reading failed or something else from the outer world, you MUST handle that exception somehow.
Of course, you can always start by simply throwing the RuntimeException as above, but that should only be a temporary first solution.
If however a File reading failed or something else from the outer world, you MUST handle that exception somehow.
Of course, you can always start by simply throwing the RuntimeException as above, but that should only be a temporary first solution.
Use Immutables
Always declare your fields final initially, and try to keep this wonderful modifier at all costs, since it helps to make your code understandable.
In general, it's a good idea to separate between immutable "value" objects which hold your abstract data, and "state" objects which have only internal fields which are not accessible, but know how to process these values.
This is a natural functional way to split (immutable) data from logic and makes your programs very fast (as immutables can be shared), easy to maintain and quick to understand at compile-time. Also, this solves tricky problems which can occur when defining hashCode on mutable fields..
This is a natural functional way to split (immutable) data from logic and makes your programs very fast (as immutables can be shared), easy to maintain and quick to understand at compile-time. Also, this solves tricky problems which can occur when defining hashCode on mutable fields..
Simple rule: Mutators (Setters) are evil.
Another rule. Immutables are not SLOW and not a matter of performance. You can always come up with fast immutable algorithms when needed. Do not sacrifice the maintainability of immutables just for the sake of performance (like the JDK did with its collections..)
Builders for Immutables
Sometimes, it its impossible to initialize an Immutable class just with the constructor - or it is clumsy and error-prone, since that "constructor" has a dozen or so fields, and you do not want to separate the object into smaller units.
In these situations, do not define setters as a last resort. Instead, define an inner class called Builder which allows it create objects of that class more easily. By convention, all method names of builders are those of the properties which are set, and a builder will always return a reference to itself by all calls (to allow method-chaining)
Other useful rules are to define no getters on them (builders are read-only), and a toString method which builds the object and then outputs it with toString (for testing).
class Foo{
public static class Builder{
private String name;
private int value;
public Builder name(String name){
this.name = name;
return this;
}
public Builder value(int value){
this.value = value;
return this;
}
public Foo build(){
return new Foo(name, value);
}
/** Common toString operation for all builders.*/
public String toString(){
return build().toString();
}
/** Common toString operation for all builders.*/
public String toString(){
return build().toString();
}
}
public static builder(){
return new Builder();
}
private final String name;
private final int value;
private Foo(String name, int value){
....
(constructor body, getName, getValue)
}
Obviously, using the builder pattern is very verbose.
Its definitely not suited for simple beans as the one i have described here, but rather for more complex scenarios when you have to initialize immutable beans with collections. However, its better to be verbose (java beans are that anyway) than writing setters which make your code harder to understand and test.
Equals / Hashcode
Equals / Hashcode
- Do not define equals / hashCode on mutable fields
Never include mutable fields into your hashing / comparision methods, as changing the values in-place will screw all HashSets / Maps where those values are resided in.
Also, its very hard to understand code which compares values with mutable states, as these comparisons could yield unexpected results in a multi-threading scenario.
- Use it only for "natural comparison"
Nearly as important: Do not define equals / hashCode for anything else than a "natural" comparision of values in collections. For anything else, write your own named methods for "special" comparisions. There are FEWER applications for hashcode/equals than most programmers think.
- Do not write your own custom logic
Rely on ides to build it for you by defining the fields. Everything else is very hard to read. One exception: Commons-lang has hashCodeBuilder and equalsBuilder, which can be both used for this comparison.
- Implement proxies for equals / hashCode, when needed
If you have a value with mutable fields which should be put into a collection... than create a mutable "proxy" class which wraps a mutable instance. This can also be used if you want to use different "equals" implementations on a class for using it in different collections. The exact semantics depend greatly on your use case. The most common idiom are "KEY"-Classes for maps, which combine different values (strings, ints) to use them as one hashKey.
- All statements said before also apply on "Comparable" classes.
Overwrite toString and use it only for logging
ToString helps the developer (that is you) during a debug session.
Because of that, you should nearly always provide it with all fields of the object. If you do not want to write it by hand, you can use the ToStringBuilder of commons-lang, which provides a toString implementation which is based on reflection.
Never use it for functional parts of your application, except for immutable values like Strings, Integers, ..
For mutable values, always the reflective:"show-all" mechanism of commons-lang should be used instead.
