CodeSOD: Inside a Box

Yesterday, among many many problems in that gigantic switch statement, we highlighted one line that was a real mess of mixing Java's primitive/object types.

(new Integer(1).equals(job.getSource().getLength()))

Well, Daniel has an even worse example, but this one comes with an origin story.

Many, many years ago, in the early 2000s, Daniel worked for a company that made banking software. As this was the early 2000s, their flagship product remained a text-based UI accessed via a terminal emulator (or, in some rare cases, even dumb terminals). Obviously, continuing this TUI approach wasn't going to work in the future, so the company needed to pivot to a Java-based web application.

The CEO set the direction: "We need a modern, web-based interface to sell banks. How much would it cost to implement that?" Well, Daniel's team put together the estimate, which contained a large number of trailing zeroes before the decimal point. Too many, from the CEO's perspective, so the CEO went shopping for a third-party developer who could deliver the software on the cheap.

Again, this was the early 2000s, which meant the CEO was filtering through a dozen different obscure offshoring companies. The CEO wasn't impressed by most of them, but one offshore company came to the meeting with a wireframe of the application built in PowerPoint. The CEO was impressed that they already had a UI where you could click buttons and navigate to the next screen, and took this to mean that they were basically done already. He wrote them a check for an amount in the millions, and off they went to work.

Many months later, the offshore team handed off the code. The first time, it didn't build. The second time, what unit tests there were didn't pass. The third time, it didn't build again. By the fifth time, the offshore team handed them Java code which would build and run, and only crash unexpectedly some of the time.

This code used certain... patterns. The same basic approaches to doing simple tasks were reused everywhere, because clearly this code base was implemented by copy-pasting snippets from other projects, from elsewhere in this project or from the first hit on a search result.

On such pattern was how they handled passing primitive types around:

int x = new Integer( 0 ).intValue();someMethod( new Integer( x ) );

Talk about thinking out of the box. "Boxing", of course, being the name for the operation where you convert a primitive type to an object type, something Java can do mostly transparently in the first place. And while misusing boxing is a frequent Java anti-pattern, this one really doubles down on it.

We box the literal 0 as an object, then get its primitive value to store it as a variable, then we box the primitive variable as an object again. And someMethod was almost always defined in terms of a primitive type, so Java automatically unboxes that object back into a primitive.

Daniel adds:

And these automaton programmers copy/pasted this little piece of gold all over the system... it was nice identifying it and correcting it, but then of course, they'd blame us whenever something went wrong because we touched their original code.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.