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;
}
});
}
2 Answers 2
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.
-
3And extracting the body into a new function with its own scope would justify the shorter variable name that you want.Kilian Foth– Kilian Foth2015年11月12日 14:33:43 +00:00Commented Nov 12, 2015 at 14:33
-
6Hmm - 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 in Orbit– Lightness Races in Orbit2015年11月12日 21:53:33 +00:00Commented Nov 12, 2015 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– CodesInChaos2015年11月16日 11:00:49 +00:00Commented Nov 16, 2015 at 11:00
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.
-
1came for the {} block, +1ed for the lambda utilityseldon– seldon2015年11月12日 21:14:59 +00:00Commented Nov 12, 2015 at 21:14
WITH
saves you typing effort by implying theb
prefixed to every unresolved symbol, but here you're still typing theb
manually everywhere.