CodeSOD: It’s Not None of Your Business is True

Patricia's employer hired her because she knew Python. They were a Python shop, at least at this point, but they'd never actually hired a developer with Python experience before.

At some point, their PHP developer wanted to branch out, and decided to solve a problem using a small Python script. It was very successful, and features got added to it. In a surprisingly short amount of time, the script expanded until it was two separate web applications which were absolutely critical to business operations.

It was a bit of a mess, when Patricia took over. The code was split across three source control repositories. One was for "Application A", in its entirety. One was called "Application B Backend", which as the name implied was the backend of Application B. The third was called "Application B" and was described as the "front end" but it was actually a copy of "Application B Backend".

It was 40,000 lines of copy-pasted nonsense, and the core of it was a module called "gsf" or "general standard functions". It was almost all of the shared logic across modules. It was designed so that it never raised an exception, and instead every method that could possibly have to deal with an exception condition instead returned a tuple: (actualReturnValue,successBoolean). Thus, you could invoke something like this:

entity, succesful = gsf.getCustomerByPrimaryKey(primary_key) if successful: …

Now, that'd be annoying but tolerable. Definitely not a WTF. But that's not how this code got used. Instead of that, the developer wrote it like this:

entity, succesful = gsf.getCustomeryByPrimaryKey(primary_key) if not (entity is None) == True: # hundreds of lines of code... else: gsf.saveLogInDatabase("Something went wrong") return

You see, if the operation failed, successful would be false, but the entity returned would also be None.

Now, the Pythonic way to write that would be to say: if entity:- None is always "false-y". Instead, the developer responsible found the most convoluted way to express the idea, ignoring the perfectly good boolean value they could have used in the first place, ignoring the behavior of None values, and instead just… spamming operations.

The preferred way would be to throw exceptions. The second best would be to if successful, then if entity, then if entity is not None, then if not (entity is None), and if you'd just consumed a vat of rubbing alcohol and held your breath for five minutes, finally your choice would be if not (entity is None) == True.

So yes, I hate that, but let's not ignore the absolute glory that is the else error handler. I'll let Patricia summarize:

The error message "Something went wrong"t was not printed to a logger or to stdout or to a file, but to a database table that contained about two million rows of the sentence "Something went wrong"

I'm just surprised that it was only two million, and I assume someone truncated the table.

Oh, there is another surprise here. Again, I'll let Patricia explain:

Luckily for me, the software manager understood my plight and let me rewrite everything from scratch.

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

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published. Required fields are marked *