6

I fooled around with for-loops, remembered the with keyword from delphi and came up with following pattern (defined as a live template in IntelliJ IDEA):

for ($TYPE$ $VAR$ = $VALUE$; $VAR$ != null; $VAR$ = null) {
    $END$
}

Should I use it in productive code? I think it might be handy for creating temporary shortcut variables like these one-character variables in lamdas and couting for loops. Plus, it checks if the variable you are going to use in the block is null first. Consider following case in a swing application:

// init `+1` button
for (JButton b = new JButton("+1"); b != null; add(b), b = null) {
    b.setForeground(Color.WHITE);
    b.setBackground(Color.BLACK);
    b.setBorder(BorderFactory.createRaisedBevelBorder());
    // ...
    b.addActionListener(e -> {
        switch (JOptionPane.showConfirmDialog(null, "Are you sure voting +1?")) {
            case JOptionPane.OK_OPTION:
                // ...
                break;
            default:
                // ...
                break;
        }
    });
}
  • 7
    What does this achieve? WITH saves you typing effort by implying the b prefixed to every unresolved symbol, but here you're still typing the b manually everywhere. – Kilian Foth Nov 12 '15 at 14:14
  • The variable is dedicated to be used in its own scope, abbreviates + chaches a possibly lengthy expression and performs a null-check. I am aware that java does not support this, thus you cannot get rid of the explicit calls to the temporary variable - but you can give it at least a short(er) name. – Binkan Salaryman Nov 12 '15 at 14:20
  • 8
    Why not simply a local variable in a nested scope? – CodesInChaos Nov 12 '15 at 14:26
  • what?! - as a serious response.... – AK_ Nov 17 '15 at 18:55
33

It's not an anti-pattern, because that would mean it is a commonly used technique that's problematic somehow. This code fails to meet the "commonly used" criterion.

However, it is problematic. Here are some of its problems:

  • Misleading: it uses a loop structure, but never executes more than once.
  • Hiding a check: the != null check is easy to miss where it is placed. Hidden code is harder to understand. It's also not clear whether the condition is actually necessary or just there to terminate the loop after the first iteration. (Your statements about the check indicate that you have situations where it's necessary, whereas in your example it's not, as new never returns null.)
  • Hiding an action: the add(b) statement is hidden even better. It's not even sequentially at the position where you'd expect it.
  • Unnecessary: You can just declare a local variable. If you don't want its name to be visible, you can use a block statement to limit its scope, although that would be an anti-pattern (or at least a code smell), indicating that you should extract a function.
  • 3
    And extracting the body into a new function with its own scope would justify the shorter variable name that you want. – Kilian Foth Nov 12 '15 at 14:33
  • 6
    Hmm - I use blocks fairly generously to limit scope, not because I have to but because I just like to be tidy. In a lot of those cases I really don't feel a separate function is warranted. But I hadn't thought about this possibly being an anti-pattern. Interesting point I'll spend some more time thinking about in future. – Lightness Races with Monica Nov 12 '15 at 21:53
  • 2
    @LightnessRacesinOrbit I think code smell fits better. It's not a problem per se, but hints at a design problem (you function might be too long). Personally I think that there is a place for this pattern, especially if you have enough shared variables to make a separate function annoying. – CodesInChaos Nov 16 '15 at 11:00
29

What you are doing is obfuscating your code, not making it clearer. It takes a lot of effort to understand that this “loop” will only execute once. Instead, you could just use a statement block to restrict the variable's scope:

{
    JButton b = new JButton("+1");
    b.setForeground(Color.WHITE);
    ...
}

If you're into lambdas, you could also define a static with function that allows usage such as

with(new JButton("+1"), b -> {
    b.setForeground(Color.WHITE);
    ... 
});

This may be a bit clearer, but is still a lot of indirection compared with the simple block that achieves the same thing, without any chance of introducing new errors.

  • 1
    came for the {} block, +1ed for the lambda utility – mattecapu Nov 12 '15 at 21:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.