Every developer eventually breaks something in production. Whether by negligence, ignorance, or just plain bad luck, it happens even to the best of us. "Oops" moments like these are the ones we learn and grow from. The broken thing gets fixed, some data may or may not get cleaned up, and everybody moves on a bit for the wiser.

Unfortunately for Matt, he doesn't get to take a mulligan; he works in the industry where bad code might not cost just money and time, but lives as well. His organization has a very good reason for putting in place a strict code review process ahead of every promote.

So, when one of the newest developers, Doug, made changes to a module which Matt had written, it only made sense that Matt review the changes. He expected it to go well; Doug was an industry veteran with decades of experience with sterling references.

The first thing that Matt noticed was the lack of any comments or documentation about how the code was intended to work. Bad, but not a huge deal; catching things like this is one of the reasons why code reviews exist.

The second thing that Matt noticed was Doug's interesting approach to a common problem. Sometimes he wanted to do one thing IF a condition was true, or ELSE do a second action. A lesser coder might just have used an if/else construct, but not Doug. He had decades of experience. 


if (a)
{
  doSomething();
}
else if (!a)
{
  doSomethingElse();
}

Rather than use a straight "else", Doug inverted the condition and used an "else if". Doug would do this regardless of the complexity of his conditional statements, and occasionally, he made a few mistakes: 


if (a < 10 && b >= 30 && c != null)
{
  myFunctionA();
}
else if (a > 10 || b < 30 || c == null)
{
  myFunctionB();
}

Doug was a go-getter. He made the effort to go through the rest of the module and bring all the if/else statements into his if/else if "pattern". As a result, no fewer than ten bugs were created in the process.

Since Doug was new to the organization, Matt saw this as an opportunity to discuss best practices and for Doug to learn from the process. However, when Matt suggested that his approach may not have been the best approach in the code review, Doug refused to back down. After all, he had decades of experience.

"But if I just use an else," he asserted, "then ANYTHING could happen!"

Matt tried pointing out that adding the "else if" doubled the complexity of each if statement, doubling the chance of errors. This didn't dissuade him. Nor did pointing out that, by creating code paths that did not assign these variables, he would cause unpredictable behavior depending on the prior values of the variables - test cases might pass or fail based on luck.

Doug adamantly refused to make the changes that Matt asked, and Matt refused to sign off to allow his development to advance.

Luckily for Matt and the customers, management stepped in to end the stalemate. Doug is now the newest ex-developer, and the code that he wrote will never see the light of day, but don't feel bad for him. He shouldn't have a problem finding work - he has decades of experience.

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