Friday, November 06, 2015

Wrong, but technically right

tldr;Immutable objects should have all-final fields.

I'm reading one of those "interview questions" posts and this jumped out at me:

Question 3: Does all property of Immutable Object needs to be final? (answer) Not necessary, as stated above you can achieve same functionality by making member as non-final but private and not modifying them except in constructor. Don't provide setter method for them and if it is a mutable object, then don't ever leak any reference for that member. Remember making a reference variable final, only ensures that it will not be reassigned a different value, but you can still change individual properties of object, pointed by that reference variable. This is one of the key point, Interviewer like to hear from candidates.

This is unfortunate. Although technically true, in my experience I've found the non-final case to come up <1% of the time. It's terribly misleading for less experienced developers to see an answer like this without having the context that goes with it.

To understand why I'm trepidatious, you can't go wrong with the C2 wiki on value objects. The right pattern for immutable objects in Java goes something like:

@RequiredArgsConstructor(staticName = "valueOf")
public final class HiMomImImmutable {
    public final int i;
    public final String s;

Here I use Lombok annotations as shorthand for the equivalent Java code. If you're not familiar with Lombok, please become so—it's both a time-saver and a bug-killer in your code.

Lombok has a @Value annotation for these. However I disagree that with their choice that fields should be private and accessed with getters. To me this makes sense when consuming frameworks requiring bean-like getters, but not otherwise.

Back to the post, the author's answer is unclear: he mentions modifying final fields in the constructor. But of course, any field marked final may be assigned in the constructor—must be so assigned—if not initialized in declaration. Then if you have complex logic to initialize (itself a code smell), try this:

public SomeClass(/* args */) {
    this.someField = aStaticMethod(/* some other args */);

This encapsulates the initialization logic in a static method as the final field may be initialized only once, and not referred to before initialization.