try {
if (foo())
return 1;
} catch (Exception e) {
throw e;
} finally {
return 3;
}
The questions I start with are:
- what does this do if foo()==true? If foo()==false? If foo() throws an exception?
- how could you recode this more simply?
- the original spec was:
- if foo is true, return 1,
- if foo is false, return 3,
- propagate exceptions.
It's sobering to note that over 2/3 of interviewees for senior Java programmer positions fail all three questions. How would you answer them?
I firmly believe that even mid-tier developers should have no problem describing what they mean in code, and in understanding what others mean, even in code which is poorly written. There always seems to be a snarl somewhere that nobody wants to touch (I've written one or two of those myself). For the most part, though, the bad code I see is the result of "I'm not sure how to do this, but this seems to work".
Here's one such snippet:
static final BigDecimal ONE_HUNDRED_PERCENT = new BigDecimal("1.00")
.setScale(MathUtils.DOLLAR_SCALE, MathUtils.STANDARD_ROUNDING_MODE);
This is just bad code, unless you're letting BigDecimal manage all the results' scales itself (we aren't, and you shouldn't; see my previous article on BigDecimal rounding). It's completely replaceable with "BigDecimal.ONE". It leaves the resulting code less clear, and performs no useful function.
I found the following code (paraphrased) in a Java application I'm working on:
foo(vc.getNewValue(), ppCorrectedValue=vc.getNewValue());
It's perfectly correct and exactly equivalent to:
ppCorrectedValue = vc.getNewValue();
foo(vc.getNewValue(), ppCorrectedValue);
That sort of expression is common in C; most C developers are aware that the equals operator evaluates to the left-hand value of the expression. Many Java developers aren't aware of this fact--so I was surprised to see it. I'm not sure it improves the readability of the code, though, so I'm refactoring it into the second form above.
Where I think the equals operator behavior really helps in Java is when setting up initial values:
i = j = 0;
Good code is hard enough to write; have pity on the next guy, and be as straightforward and clear as possible. Arabesques like assignment inside a function call parameter list just make your code harder to read and maintain.
No comments:
Post a Comment