CodeSOD: Getting Reported

Monica sends us a code snippet that comes from her organization's home-brewed reporting engine. She writes:

I have always found that "Homebrew Reporting Engines" are always full of WTF material. For some reason, coded reports are a "good place to learn the codebase" for junior engineers and usually have deadline of "just get that done".

In this case, a Java web application needs to load a list of reports. For some reason, the list is stored in the web.xml file, even though all the data about the reports is actually stored in the database. That's a nuisance, but this is the code which fetches it:

Boolean bool = Boolean.valueOf(!Toolkit.isEmpty(list = WebConfigInitializer.WebConfig.instance().getConfigValues("Reports", hs))); request.setAttribute("multipleReports", bool); if (bool.booleanValue()) { ArrayList<RepDefinitionBean> arrayList = new ArrayList(); for (Iterator<String> iterator = list.iterator(); iterator.hasNext();) { if ((reportList = ReportListApp.loadByName(str = iterator.next())) != null && ( reportDefinition = ReportDefinitionApp.loadReportDefinition(reportList.getReportNo())) != null) { RepDefinitionBean repDefinitionBean = new RepDefinitionBean(str, reportDefinition.getReportLabel()); arrayList.add(repDefinitionBean); } } request.setAttribute("reports", arrayList); }

This seems to be an exercise in writing this code in the least comprehensible way possible. Mixing assignments with access- isEmpty(list = …), for example. There's the spurious Boolean.valueOf, when we know isEmpty has to be returning a bool since we see the ! operator. And why box the bool in the first place, when we're just fetching its booleanValue() a few lines later? Then of course the for loop which uses an iterator, but doesn't use the for (String str : list) syntax, which was definitely available in the version of Java this code was written under. Or the conditional which mixes multiple variable assignments with null checks all in one big step.

What got Monica looking at this code, however, was trying to understand why a new report wasn't available. Of course, you'll note those null checks- the person who developed the data access layer chose not to raise exceptions and instead returned nulls when data couldn't be found. So someone misspelled a report name in the web.xml and thus the report couldn't be run, but there were no errors to indicate what might have gone wrong.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published.