CodeSOD: Accessed Nulls

"The attached class connects to an Access database," writes Nicolai. That's always a good start for a WTF. Let's take a look.

public class ResultLoader {private static Logger sysLog = Logger.getLogger(ResultLoader.class);private static String url = "somePath";/** * get the ResultTable from the Access database * * @param tableName * @return */private static Table getResultTable(String tableName) {try {// create a new file with the path to the tableFile db = new File(url);// let Jackcess open the file and return a tablereturn Database.open(db).getTable(tableName);} catch (IOException e) {e.printStackTrace();}return null;}/** * load result from DB */public static void loadResult() {String tableName = "Result";Table resultTable = getResultTable(tableName);if (!resultTable.equals(null)) {Map<Integer, Float> yearConsumption = new HashMap<Integer, Float>();for (Map<String, Object> row : resultTable) {/* * [snip] does something with the table's rows */}Result result = new Result(00, new Date(), consumptions);} else {sysLog.info("There is no data object in the Access Database!");}}}

Now, this code was written by a warm body- the company just found someone who "could code a little" and told them to get to work. So I wouldn't blame the developer here- they were doing their best. But the company failed them, by letting this run into production.

Which, with that in mind, this code isn't terrible. Oh, the hard-coded URL for the database is a mistake, sure. Similarly for the hard coded table (at least make a constant somewhere convenient).

In fact, there's barely anything in here that really merits a WTF. Barely.

!resultTable.equals(null)

If getResultTable fails, it doesn't return anything. So resultTable will be null. But if it's null, we can't process anything, so we need to do a null check. And this poor developer, thrown into a project they were ill prepared for, did their best. They understood that in Java, you overload the equals function, and should generally prefer that for testing for equality, but didn't understand the idea of testing for identity- so they used equals.

The used a member function of the class. On a variable which may be holding null. This, of course, doesn't work. If getResultTable failed, this line will throw a fresh exception.

As an aside, this is why I always recommend being lazy about handling exceptions- exceptions should propagate up to a level at which they can be handled meaningfully. Don't return nulls.

The poor developer responsible for this has long since moved on, and graduated from "can kinda code a little" to being a full-fledged developer. Nicolai, on the other hand, has to suffer for the decisions made by management past, and fix these problems.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!