Welcome back to our continuing series called Ways Matthew Has Screwed Up.  I'm thankful that today's installment is a relatively minor issue, and am reminded of how important it is to just keep moving.

The Grand Refactoring

My group is in charge of several large apps, many of which are in dire need of significant updates.  Hey, it's part of working for a large company, so most of the time I don't mind.  But one of the apps we control, the largest one in fact, was so old, slow, and stupid that we collectively decided it needed a substantial refactoring effort.  We termed this effort the Grand Refactoring, and it started about a month ago.

man on white ladder
"No worries boss, we're about 90% done." Photo by Henry & Co. / Unsplash

For the most part, the Grand Refactoring has been going smoothly; but veterans of refactoring will tell you that's always what happens at the beginning.  As we get closer to completion, we start having integration problems.  Some of the new repositories don't work nicely, some have circular dependencies, some simply weren't designed in the manner we need them to be.  It's painful to fix, but we are getting it done.  It's just slower than we'd like.

The particular thing I screwed up was representative of the kinds of problems we've been having.  See, I saw the following LINQ statement:

var alerts = _ourEntities.Alerts.Where(x => (x.PartA == currentPartA && x.PartB == currentPartB && x.PartC == currentPartC) || (x.PartA == currentPartA && x.PartB == currentPartb && x.PartC == null) || (x.PartA == currentPartA && x.PartB == null && x.PartC == null))

Look at all those null checks!  Surely they aren't necessary!  I figured this could be simplified to the following:

var alerts = _ourEntities.Alerts.Where(x => (x.PartA == currentPartA && x.PartB == currentPartB && x.PartC == currentPartC) || (x.PartA == currentPartA && x.PartB == currentPartB) || x.PartA == currentPartA)

Much better.  No way this could cause issues.  This plan is foolproof!

When I tested this, it appeared to work fine.  The alerts were being picked up, and all was well.  So I went through code review, built the project, everything looked good.

But the problem remained.  It lay in wait for the perfect moment, and that moment occurred when my teammate Hafsa got ahold of my changes.

NULL Doesn't Mean Nothing?

Hafsa was trying to solve a problem totally unrelated to mine, but something wasn't working right.  She kept getting duplicate Alerts where none should exist. Eventually, she tracked it down to my code.  She found the fix, implemented it, and sent it to me for code review.

Which is when I saw this:

var alerts = _ourEntities.Alerts.Where(x => (x.PartA == currentPartA && x.PartB == currentPartB && x.PartC == currentPartC) || (x.PartA == currentPartA && x.PartB == currentPartb && x.PartC == null) || (x.PartA == currentPartA && x.PartB == null && x.PartC == null))

Hafsa put back the exact same LINQ statement I had tried to "simplify".  I had to go and talk to her about it and she pointed out the flaw in my reasoning: by taking out the NULL checks, I had guaranteed that any matching item would be returned multiple times.  The system we were refactoring didn't mean for NULL to represent nothing; it was a distinct value and meant that the alert in question merely didn't have that Part.  I was dumbfounded.  

brown and black cat
"Surely you don't think it was little ol' me?" Photo by Paul Hanaoka / Unsplash

As is the root problem with most of this series, I just didn't do enough research.  Not that I'm ever going to be perfect, but I'm pretty proud of my ability to research deep enough into problems so as to solve them thoroughly and correctly the first time.  I just... screwed up this time.

But it's no big deal.  It has to be no big deal.

We Keep Moving

Lucky for Hafsa and myself, we caught this bug before it caused major annoyances (the nature of the bug being that it probably wouldn't cause any more harm than invoking a stern lecture from our business units).  I wasn't happy about it, of course.  But I am forced to keep moving on.

After all, if I was going to let one screwup screw me up, the Grand Refactoring wouldn't be nearly done.  If I was going to let screwups affect me so badly, to be frank, I shouldn't be a developer.

Learning to write code is learning to fail, and to keep moving in spite of said failure.  We plan, we write, we test, we refactor, we fix, we deploy, and inherent in all of this is the inevitability of mistakes.  We developers are wrong many, many times a day.  But we keep moving.  We have no choice.  Technology, and our jobs, aren't going to wait for us to get ourselves together.  We keep moving, for fear of falling behind.

I screwed up.  I own that.  I have no idea why this particular bug caused this particular reaction in me.  I'm fortunate that it was a comparatively minor issue.  And above all, I keep moving.

Don't stop moving.