Wood for the Trees

ActiveTest: Examination, Part III: Code Bloat

Posted by Mathew Abonyi Tue, 02 Jan 2007 03:39:28 GMT

Another problem you can see from the final example in Part II is a hint of code bloat. I somehow managed to add way more lines of code to just define my original test case of 3 lines. Along the way, I made it very difficult to document what any single method did, because they all provided only pieces. If you wanted to find out what succeeds_on :index actually does, you’ll find yourself trawling through more than a handful of methods. This is the terror of too much premature extraction.

Each Subject received the Spartan method ‘define_behavioral’, whose function is solely to ensure a new method’s name is unique:

      def define_behavioral(prefix, core, specifics = {}, &block)

        proposed = "test_should_#{prefix.to_s.underscore}"
        proposed << "_#{core.to_s.underscore}" if core.respond_to?(:to_s)

        while method_defined?(proposed)
          if proposed =~ /^(.*)(_variant_)(\d+)$/
            proposed = "#{$1}#{$2}#{$3.to_i + 1}" 
          else
            proposed << "_variant_2"
          end
        end

        define_method proposed, &block

        proposed
      end

The bloat is in determining such nondescript method names. It is a result of the poor design that any dynamically generated method would clash with another one. Ultimately, it is better if the developer defines the name of the method, because it would be used in a rake agiledox call. After all, it is their documentation. It did not help matters that this problem was glossed over by writing my own agiledox which loads in everything first, then uses ObjectSpace… another rather inelegant hideous solution.

If we remove the unique naming, define_behavioral is useless; it just wraps define_method. Also, if we remove the dynamic call_#{action}_action and instead use an instance method on ActiveTest::Controller, we reduce method_missing and drop a bunch of unnecessary methods:

class << self
  def succeeds_on(method_name, action, options = {})
    request_method = determine_request_method(action)
    define_method(method_name) do
      call_request_method request_method, action, options
      assert_response :success
      assert_template action
    end
  end
end

Using a direct definition removes the flexibility, but all the code bloat is about meeting some unmentioned need of customising. Until an e-mail is received about it, why do it? Also, better solutions which apply to more than just ActiveTest::Controller may appear naturally. All in all, better not to be so flexible so prematurely.

What Was I Thinking?

I really wanted to write a flexible and easily customised macro language. I thought I could make a clean, elegant macro writer for macro writers, with which test cases were created flexibly and efficiently. The flexibility is what caused all the bloat because I decided to use the expedient of method_missing rather than design a register_behavior type of declaration. Also, I thought it would be better to cater to more edge cases than pigeon-hole ActiveTest to only the most common cases (which could potentially be very few, making ActiveTest pretty useless). The mistake was in trying to cater to these two exclusive cases (common and edge) in their specifics (what they include) rather than how they operate (make a request and check a response).

Coming Up Next: Salvaging Useful Ideas…

Posted in Discoveries, Ruby, Rails, Plugins | no comments | 2 trackbacks