CodeSOD: You Can Take Care

Tiberrias sends us some code that, on its face, without any context, doesn't look bad.

var conditionId = _monitorConditionManagement.GetActiveConditionCountByClient(clientIdentityNumber);_monitorConditionManagement.StopCondition(conditionId);

The purpose of this code is to lookup a condition ID for a client, and then clear that condition from a client by StopConditioning that ID. Which, if you read the code closely, the problem becomes obvious: GetActiveConditionCountByClient. Count. This doesn't return a condition ID, it returns the count of the number of active conditions. So, this is a stupid, simple mistake, an easy error to make, and an easy error to catch- this code simply doesn't work, so what's the WTF?

This code was written by a developer who either made a simple mistake or just didn't care. But then it went through code review- and the code reviewer either missed it, or just didn't care. It's okay, though, because there are unit tests. There's a rich, robust unit test suite. But in this case, the GetActiveConditionCountByClient and the StopCondition methods are just mocks, and the person who wrote the unit test didn't check to see that the mocks were called as expected, because they just didn't care.

Still, there's an entire QA team between this code and production, and since this code definitely can't work, they're going to catch the bug, right? They might- if they cared. But this code passed QA, and got released into production.

The users might notice, but the StopCondition method is so nice that, if given an invalid ID, it just logs the error and trucks on. The users think their action worked. But hey, there's a log file, right? There's an operations team which monitors the logs and should notice a lot of errors suddenly appearing. They just would have to care, which guess what"

This bug only got discovered and fixed because Tiberrias noticed it while scrolling through the class to fix an entirely unrelated bug.

"You really shouldn't fix two unrelated bugs in the same commit," the code reviewer said when Tiberrias submitted it.

There was only one way to reply. "I don't care."

[Advertisement] ProGet supports your applications, Docker containers, and third-party packages, allowing you to enforce quality standards across all components. Download and see how!