Assign Once

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;
    }
}

Localize Scope

Here’s another variant:

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.