Today, we'll talk on the virtues of restraint, cleverly disguised as an exercise in TDD.

Here's the bug that I snagged during the stand-up: When I try to delete a Mechanical Doohicky*, I get an unhandled exception.

Careful and value-added analysis led to the following code that checks to see if the Product is being used by another object:

        public bool isPartUsed( IMechanicalDoohicky part )
        {
            foreach ( Heap heap in heaps )
            {
                if ( heap.Engine.Parts.Contains( part ) )
                {
                    return true;
                }
            }
            return false;
        }

The issue should be readily apparent. Namely, heap.Engine might be null.

My first reaction to this was: easy fix. Check heap.Engine for null and spend the rest of the hour playing Whack-A-Mole (not the version you're thinking of).

But nay, coders, let's slow down and see if there's a way we can drag this billable time out a bit in a justifiable way**.

What we're going to do first is write a test to expose this little bug. For those of you who obsess over analogies, think of it as...oh, I dunno, fixing a leak in a dam. You don't want to shut the water off, then have to turn the water on to make sure the leak is fixed. OK, maybe I'll pass on the analogies. Hillbillies are just too real. But I'm writing the test anyway because it's trendy.

Over to our test harness, we add a test that will fail specifically because of this bug:

        private bool Should_not_fail_when_checking_if_part_is_used_in_heaps_with_no_engine( )
        {
            // ... set up part and heaps with one that has no engine ...
            heaps.isPartUsed( part );
        }

Back to our code and we update it to fix the bug:

        private bool isPartUsed( IMechanicalDoohicky part )
        {
            foreach ( Heap heap in heaps )
            {
                Engine engine = heap.Engine
                if ( engine != null )
                {
                    if ( engine.Parts.Contains( part ) )
                    {
                        return true;
                    }
                }
            }
            return false;
        }

So it may seem like a lot of extra work to fix a seemingly simple bug, especially because of the ominous way I commented out all the set up code in the test. But the bug is fixed and I can now prove it without having to launch the UI and going through the pain of setting up the conditions that led to it. And if anyone mucks with the implementation of Engine, we can be reasonably confident this particular bug won't get re-introduced. Plus the test acts as a sort of documentation of the bug that we've fixed.

Final note: There is a reason I stored heap.Engine in a temporary variable other than avoiding duplication.

Kyle the Exposed

* Domain objects have been renamed to protect the innocent. I.E. me

** My lawyer has advised me that, due to my increased readership in the U.S., I should add that I am officially and legally kidding.