Saturday, February 21, 2015

Case Study: TDD Is Worth It!

   One of my most popular blog posts, counting its reading elsewhere, is Is TDD Worth It?  Recently I had a great example of how, to quote myself, "It's almost like giving you guard rails."

   Suppose the application domain is as follows:
  • Things have Parts.
  • Parts have Types, which have Categories.
  • Within a given Thing, Parts are used with various specific Connectors.
   I had been asked to develop functions to find the Things that use a given Connector, and:
  • Parts of a given Type or Category.
  • Parts of any Type or Category out of a set of Types or Categories.
  • Parts of all the Types or Categories out of a set of them.
  • The negatives of all of the above, i.e., those using:
    • no Parts of that Type or Category,
    • no Parts of any Type or Category of a set,
    • and those not using Parts of all (though they may use some) Types or Categories of a given set.
   The problem is, I had been given these requirements one at a time, so I thought the client wanted a bunch of methods on Thing with names like:
  • find_with_connector_and_part_type(conn_type, part_type)
  • find_with_connector_and_any_part_types(conn_type, part_types)
  • find_with_connector_and_all_part_types(conn_type, part_types)
  • find_with_connector_and_part_category(conn_type, part_category)
  • find_with_connector_and_any_part_categories(conn_type, part_categories)
  • find_with_connector_and_all_part_categories(conn_type, part_categories)
  • find_without_connector_and_part_type(conn_type, part_type)
  • find_without_connector_and_any_part_types(conn_type, part_types)
  • find_without_connector_and_all_part_types(conn_type, part_types)
  • find_without_connector_and_part_category(conn_type, part_category)
  • find_without_connector_and_any_part_categories(conn_type, part_categories)
  • find_without_connector_and_all_part_categories(conn_type, part_categories)
   I kept the code as DRY as I could, making the negatives simply call the positives and invert the finding, and making the singles pass a single-element set to the "any" case. That still left a lot of structural duplication.  Since they were class-methods used as scopes, they all looked like this:
def self.find_with_connector_and_part_SOMETHING(conn_type,
                                                SOMETHINGS)
  id_query = assorted_magic_happens_here(several_lines)
  where(id: id_query)
  # negative version: where("id NOT IN (?)", id_query)
end
   That got me thinking about how to combine this set of literally a dozen different functions into one, something like Thing.find_with_parts(part_options), where part_options would include what Connector, what Types (and whether we want all or just any, which would be the default), what Categories (ditto), and whether to invert the finding.  When the client later said they didn't in fact want a bunch of separate methods, I was ready, and had a lot of the idea already thought out.

   But... how do I get from Point A to Point B?  This is where TDD came in handy!

   Of course, I had been TDD'ing the original functions, using the classic "red, green, refactor" cycle.  (Actually, I use a variant that adds a step: "refactor the tests".)  So, I substituted a simple call to my proposed new function, for the guts of one of the old ones:
def self.find_with_connector_and_part_type(conn_type,
                                           part_type)
  self.find_with_parts(connector_type: conn_type,
                       part_types: [part_type])
end
and reran its tests.  Of course it failed, as I hadn't written find_with_parts yet... but that came pretty easily, based on the logic that had previously found Things having Parts of any of several Types, used with the given Connector.  That test quickly passed.

   Long story short, I followed this pattern, over and over:
  1. Substitute a call to find_with_parts for the guts of a specific method.
  2. Run its test.
  3. If it works, break it by changing find_with_parts!  You always want to start with a failing test!
  4. Fix find_with_parts to make the test pass.
  5. Run the rest of the tests!
  6. If any of them fail, go back to fixing find_with_parts.
When all the old methods had had their guts substituted, I knew the new method could do it all, so I ripped the old methods out.

   Had it not been for having an existing test suite, which I had because I had TDD'ed the original code, I would have had to be a lot more slow, careful, and methodical in that process.  Instead, I could just quickly try what came to mind, and see if it worked without breaking anything else.

   The resulting function, including some extracted functions to keep that code dry, runs to a mere 43 lines of code, and is structured in such a way as to make it very easy to add additional items to filter on, such as the Material a Part Type is made of, the color a given Part is painted when used in that Thing, etc.

   (Yes, yes, I could have created the test suite just before embarking on this... but seriously, what are the chances?  Most developers would not bother, for something smallish like this.  Perhaps for a larger code archaeology expedition, where writing a test suite to ensure continuing the current behavior, whether correct or not, is a common first step.)