CodeSOD: One Way to Solve a Bug

Startups go through a number of phases, and one specific phase is the transition from "just get it done and worry about the consequences tomorrow" into "wait, maybe if we actually did some planning and put some process around what we do, we won't constantly be one step behind the current disaster."

And that's when they start to hire people who have more management experience, but are also technical enough that they can contribute to the product directly. At BK's company, the latest hire in that category is Sylvester.

Sylvester is the new team lead, and he comes from a more "enterprise" background, which means he's had a very difficult time getting up to speed, and is unwilling or uncomfortable to make decisions with limited information. And also, Sylvester might not be particularly good at the job.

BK noticed that Sylvester had a commit sitting in code review, and it had been sitting there for some time, so they took a look. One of the first things they spotted was a method called SolveBug, which made it clear they were in for a "treat".

void SolveBug (std::vector<BusinessEntity> container){ std::sort(container.begin(), container.end()); const auto it = std::unique(container.begin(), container.end()); if (it != container.end()) { container.erase(it); }}

So, one of the business rules about the vector of BusinessEntity objects is that the entries should be unique. BK skimmed this method, and without looking at it too deeply, and without knowing the definition of std::unique, made some assumptions based on the code. The std::unique function must return the first duplicate, and then we erase it.

That couldn't be right though, because what if there were multiple duplicates? So, BK did what Sylvester obviously didn't, and did a quick search. std::unique returns an iterator where consecutive duplicates are removed.

BK, being generous, assumed they were missing something, and asked Sylvester what that might be.

"Oh, thanks for spotting that! That's probably why it doesn't work."

Apparently, Sylvester had pushed code he knew was broken, and didn't mention it to anyone. He didn't ask anyone to take a look beyond submitting a code review. He didn't bother to check the documentation, and certainly didn't think about the code he was writing.

Perhaps "SolveBug" was less a description of what the method did, and more what Sylvester hoped the code review would do? In that case, the method was very accurately named.

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