Sunday, January 10, 2010

Coupling Design and Implementation

Six weeks ago or so, our development team reviewed a small design change in the way status is managed by an object. Basically, we broke one state variable into four, and thought more carefully about the state transitions allowed and expected for the class in question.

Yesterday, one of our analysts, at the prompting of one of our developers and two of our data designers, reviewed a completely new take on the same design change... none of the guys in question knew about the previous design change (they'd missed the design review). We ended up going with the previously-reviewed design.

The whole meeting and the thought that led up to it could have been avoided if there had been some mechanism for ensuring the approved design was implemented. This bit of design, like all design, is pretty much wasted if it remains in design documents.

One way to handle this would have been to somehow automatically compare the existing design artifacts to the implementation to see if they matched, and complain if they didn't. Even if we didn't implement the approved design right away, then, there'd be a mechanism for reminding developers that they planned to do something one way, and haven't made it happen yet.

Thursday, January 7, 2010

Three snippets of "interesting" code

Here's a bit of code I use in interviews:

try {
if (foo())
return 1;
} catch (Exception e) {
throw e;
} finally {
return 3;
}

The questions I start with are:
  1. what does this do if foo()==true? If foo()==false? If foo() throws an exception?

  2. how could you recode this more simply?

  3. the original spec was:
    • if foo is true, return 1,

    • if foo is false, return 3,

    • propagate exceptions.

Provide code to implement the spec.

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.