Java Anti-Patterns – Is Using For Loop Syntax for a ‘With’ Block an Anti-Pattern?

anti-patternsjavaloops

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

Best Answer

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.
Related Topic