CodeSOD: For Mere Mortals

Repentinus submitted a pull request to a project he was working on. The Python code Repentinus submitted was this:

if unit.is_translated() and (self.include_fuzzy or not unit.is_fuzzy()): return unit.targetelse: return unit.source

Now, this may not be the absolute most readable option here, but it's pretty clear, even without knowing the domain. We return the "target" if our object is_translated, but only if it is either "not fuzzy" or we've checked the box to "include fuzzy" options. Otherwise, we return the source. It's a fine solution, rooted in the problem domain. There's room to quibble over how the condition is expressed, but it makes sense.

Well, it doesn't make sense to one of Repentinus's collaborators. They added a commit to the pull request with the comment "Let's make that logic readable by mere mortals".

Now, a moment ago, I said Repentinus's patch was fine, and it was certainly readable enough, but apparently I was wrong. Because this is what the collaborator turned it into, the version for "mere mortals":

if unit.is_translated(): return unit.targetif self.include_fuzzy and unit.is_fuzzy(): return unit.targetreturn unit.source

This, as you can see, is wrong. It doesn't have the the same flow at all, as the goal of the conditional in the first place was to return the translated version, *but if it was "fuzzy", we only returned it when the include_fuzzy flag was set.

The collaborator refused to budge on this change. They were absolutely convinced that they had streamlined the logic, and Repentinus was just having a "snit" because the commit comment wasn't "polite" enough. Of course, the change didn't pass the unit tests, so after an overlong comment chain, the collaborator deleted the commit and pretended like they'd never done it.

To borrow a quote: "Lord, what fools these mortals be!"

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!