I try to make as many variables as possible final
because I find it easier to reason about immutable code. It makes my coding life simpler, which is a high priority for me—I’ve spent too much time trying to figure out exactly how the contents of a variable change throughout a block of code. Of course, Java’s support for immutability is more limited than some other languages, but there are still things we can do.
Here’s a small example of a structure I see everywhere:
Thing thing; if (nextToken == MakeIt) { thing = makeTheThing(); } else { thing = new SpecialThing(dependencies); } thing.doSomethingUseful();
To me this doesn’t irrevocably express that we’re going to set the value of thing
before we use it and not change it again. It takes me time to walk through the code and figure out that it won’t be null
. It’s also an accident waiting to happen when we need to add more conditions and don’t quite get the logic right. Modern IDEs will warn about an unset thing—but then lots of programmers ignore warnings. A first fix would be to use a conditional expression:
final var thing = nextToken == MakeIt ? makeTheThing() : new SpecialThing(dependencies); thing.doSomething();
The only way through this code is to assign thing
a value.
A next step is to wrap up this behavior in a function to which I can give a descriptive name:
final var thing = aThingFor(nextToken); thing.doSomethingUseful(); private Thing aThingFor(Token aToken) { return aToken == MakeIt ? makeTheThing() : new SpecialThing(dependencies); }
Now the life cycle of thing
is easy to see. Often this refactoring shows that thing
is only used once, so I can remove the variable:
aThingFor(aToken).doSomethingUseful();
This approach sets us up for when, inevitably, the condition becomes more complicated; note that the switch
statement is simpler without the need for repeated break
clauses:
private Thing aThingFor(Token aToken) { switch (aToken) { case MakeIt: return makeTheThing(); case Special: return new SpecialThing(dependencies); case Green: return mostRecentGreenThing(); default: return Thing.DEFAULT; } }
var thing = Thing.DEFAULT; // lots of code to figure out nextToken if (nextToken == MakeIt) { thing = makeTheThing(); } thing.doSomethingUseful();
This is worse because the assignments to thing
aren’t close together and might not even happen. Again, we extract this into a supporting method:
final var thing = theNextThingFrom(aStream); private Thing theNextThingFrom(Stream aStream) { // lots of code to figure out nextToken if (nextToken == MakeIt) { return makeTheThing(); } return Thing.DEFAULT; }
Alternatively, separating concerns further:
final var thing = aThingForToken(nextTokenFrom(aStream));
Localizing the scope of anything that is variable into a supporting method makes the top-level code predictable. Finally, although some coders aren’t used to it, we could try a streaming approach:
final var thing = nextTokenFrom(aStream) .filter(t -> t == MakeIt) .findFirst() .map(t -> makeTheThing()) .orElse(Thing.DEFAULT);
I’ve regularly found that trying to lock down anything that does not vary makes me think more carefully about my design and flushes out potential bugs. It forces me to be clear about where things can change and to contain such behavior into local scopes.