CodeSOD: An Exception to the Rule

"Throw typed exceptions," is generically good advice in a strongly typed language, like Java. It shouldn't be followed thoughtlessly, but it's a good rule of thumb. Some people may need a little more on the point, though.

Alexander L sends us this code:

 public boolean isCheckStarted (final String nr) throws CommonException { final BigDecimal sqlCheckStarted = executeDBBigDecimalQueryFirstResult ( Query.CHECKSTARTED_BY_NR, nr); CommonException commonException = new CommonException ("DB Query fail to get 'CheckStarted'"); int checkStarted = -1; checkStarted = Integer.parseInt (Utility.bigDecimalToString (sqlCheckStarted)); if (checkStarted == 1 || checkStarted == 0) { return checkStarted == 1 ? true : false; } else { throw commonException; } }

At a glance, it looks ugly, but the scope of its badness doesn't really set in until Alexander fills some of the surrounding blanks:

• CommonException is a generic class for failures in talking to the database
• It is almost never caught directly anywhere in the code, and the rare places that do wrap it in a RuntimeException
• executeDBBigDecimalQueryFirstResult throws a CommonException if the query failed.

It's also important to note that Java captures the stack trace when an exception is created, not when it's thrown, and this method is called from pretty deep in the stack, so that's expensive.

And all of that isn't even the worst. The "CheckStarted" field is apparently stored in the database as a Decimal type, or at least is fetched from the database that way. Its only legal values are "0" and "1", making this a good bit of overkill. To round out the type madness, we convert it to a string only to parse it back into an int.

And that's still not the worst.

This line: return checkStarted == 1 ? true : false; That's the kind of line that just sets my skin crawling. It bugs me even more than using an if statement, because the author apparently knew enough to know about ternaries, but not enough to know about boolean expressions.

[Advertisement] Release!is a light card game about software and the people who make it. Play with 2-5 people, or up to 10 with two copies - only $9.95 shipped!