Ruby: Dwemthy's Array using Modules

Following my blog entry Underuse of Modules, an anonymous commenter asks

Can you show another example, then, of how one might implement the "magic" of Dwemthy's Array (http://poignantguide.net/dwemthy/) just using modules? I can never remember how to do this sort of thing, and if modules can make it conceptually simpler it would be most useful.

I'm not sure exactly what magic they were referring to, but I'll assume they mean what allows creature habits to be defined in class definitions. Based on that assumption, I pulled out this code from the example.

class Creature
  def self.metaclass; class << self; self; end; end

  def self.traits( *arr )
    return @traits if arr.empty?

    attr_accessor(*arr)

    arr.each do |a|
      metaclass.instance_eval do
        define_method( a ) do |val|
          @traits ||= {}
          @traits[a] = val
        end
      end
    end

    class_eval do
      define_method( :initialize ) do
        self.class.traits.each do |k,v|
          instance_variable_set("@#{k}", v)
        end
      end
    end
  end
end

class Rabbit < Creature
  traits :bombs
  bombs 3
end

Rabbit.new.bombs # => 3

Note, I removed the comments and added the Rabbit code. The Rabbit is there to ensure the magic continues to function as expected.

The version using a module isn't significantly better, but I do slightly prefer it.

class Creature
  def self.traits( *arr )
    return @traits if arr.empty?

    attr_accessor(*arr)

    mod = Module.new do
      arr.each do |a|
        define_method( a ) do |val|
          @traits ||= {}
          @traits[a] = val
        end
      end
    end

    extend mod
    
    define_method( :initialize ) do
      self.class.traits.each do |k,v|
        instance_variable_set("@#{k}", v)
      end
    end
  end
end

class Rabbit < Creature
  traits :bombs
  bombs 3
end

Rabbit.new.bombs # => 3

The above version is a bit clearer to me because it defines methods on a module and then extends the module. I know that if I extend a module from the context of a class definition the methods of that module will become class methods.

Conversely, the first example forces me to think about where a method goes if I do an instance_eval on a metaclass. By definition all class methods are instance methods of the metaclass, but there are times when you can be surprised. For example, using def changes based on whether you use instance_eval or class_eval, but define_method behaves the same (change metaclass.instance_eval in the original example to metaclass.class_eval and the behavior doesn't change). This type of thing is an easy concept that becomes a hard to find bug.

If you spend enough time with metaclasses it's all clear and easy enough to follow. However, modules are generally straightforward and get you the same behavior without the mental overhead. I'm sure someone will argue that metaclasses are easier to understand, which is fine. Use what works best for you.

However, there are other reasons why it might make sense to use modules instead, such as wanting to have an ancestor (and thus the ability to redefine and use super).

Again, it probably comes down to personal preference.

Extend modules instead of defining methods on a metaclass

In the entry Replace method_missing with dynamic method definitions I have the following example code.

class Decorator
  def initialize(subject)
    subject.public_methods(false).each do |meth|
      (class << self; self; end).class_eval do
        define_method meth do |*args|
          subject.send meth, *args
        end
      end
    end
  end
end

The context of the example can be summarized as, you want to delegate from the instance all the public methods defined on the constructor argument.

Ali Aghareza pointed out to me that defining methods on the metaclass of an instance isn't the nicest thing to do. The problem with it is that you've made it much harder for anyone else to change the behavior of the instance.

Here's a more simplified example. The following code creates a new Object and defines the hello_world method on the Object instance.

class Object
  def metaclass
    class << self; self; end
  end
end

obj = Object.new
obj.metaclass.class_eval do
  def hello_world
    "hello"
  end
end

obj.hello_world # => "hello"

This works fine; however, if someone wanted to change the way hello_world behaved, by defining the method on the metaclass you force them to make their change by redefining the method on the metaclass. The current solution does not allow you to extend modules and alter the behavior of the instance.

The following example demonstrates that extending a module does not change the behavior of an instance if the behavior has been defined on the metaclass.

class Object
  def metaclass
    class << self; self; end
  end
end

obj = Object.new
obj.metaclass.class_eval do
  def hello_world
    "hello"
  end
end

obj.hello_world # => "hello"

module Spanish
  def hello_world
    "hola"
  end
end

obj.extend Spanish

obj.hello_world # => "hello"

A better solution is to change the behavior of the instance by extending modules instead of defining behavior on the metaclass.

obj = Object.new

module English
  def hello_world
    "hello"
  end
end

obj.extend(English).hello_world # => "hello"

Now that the behavior is defined on an ancestor instead of the metaclass you can change the behavior by extending another module.

obj = Object.new

module English
  def hello_world
    "hello"
  end
end

obj.extend(English).hello_world # => "hello"

module Spanish
  def hello_world
    "hola"
  end
end

obj.extend(Spanish).hello_world # => "hola"

This solution works fine for our simple example, but it can also be applied to our first (much more complicated) example, even without knowing how to define the module. In the case of the Decorator, you can simply define an anonymous module and immediately extend it.

class Decorator
  def initialize(subject)
    mod = Module.new do
      subject.public_methods(false).each do |meth|
        define_method meth do |*args|
          subject.send meth, *args
        end
      end
    end
    extend mod
  end
end

Testing Anti-Pattern: Metaprogrammed Tests

Update at bottom
Update 2 for Saikuro reported cyclomatic complexity
Update 3 for Flog

I despise metaprogrammed tests. The problem with metaprogrammed tests is that they introduce more questions than answers. Tests are supposed to give confidence, but I don't feel very confident when I find myself asking: which assertion failed? what part of the test is wrong? in which loop, at what value, do you think the problem is?

Let's jump straight to an example. The following method on Fixnum will tell you what the letter grade is.

class Fixnum
  def as_letter_grade
    case self
      when 0..59 then "F"
      when 60..69 then "D"
      when 70..79 then "C"
      when 80..89 then "B"
      when 90..100 then "A"
    end
  end
end

50.as_letter_grade # => "F"
60.as_letter_grade # => "D"
70.as_letter_grade # => "C"
80.as_letter_grade # => "B"
90.as_letter_grade # => "A"

For completeness you may wish to test every value between 0 and 100 to ensure that no mistakes are made. Doing this the most straight forward way possible, you would define 101 tests and test every value individually.

require 'test/unit'

class GradeTests < Test::Unit::TestCase
  def test_zero_is_an_f
    assert_equal "F", 0.as_letter_grade
  end

  def test_one_is_an_f
    assert_equal "F", 1.as_letter_grade
  end

  def test_two_is_an_f
    assert_equal "F", 2.as_letter_grade
  end
end

While this would work it suffers from a few complications: it's too long to digest and it would be painfully tedious to write. You might jump to the conclusion that you ought to metaprogram the tests to resolve the previously mentioned issues.

require 'test/unit'

class GradeTests < Test::Unit::TestCase
  (0..100).each do |index|
    letter = case index
      when 0..59 then "F"
      when 60..69 then "D"
      when 70..79 then "C"
      when 80..89 then "B"
      when 90..100 then "A"
    end
    
    define_method "test_#{index}_is_#{letter}" do
      assert_equal letter, index.as_letter_grade
    end
  end
end

This solution isn't so bad at first glance. When a test fails, I can see what number I was working with, what letter I expected and what letter I actually got.

Loaded suite /Users/jay/Desktop/foo
Started
..........................................................
..........F................................
Finished in 0.024512 seconds.

  1) Failure:
test_70_is_C:32
<"C"> expected but was
<"D">.

101 tests, 101 assertions, 1 failures, 0 errors

Then I have to actually figure out what is wrong, and this is where I begin to really dislike metaprogrammed tests. The line number is almost worthless. Yes, the loop is on or near that line, but the actual failure isn't found exclusively on that line, it also contains about 100 successful assertions. Also, I always expect the problem to be in the class, but that's not always the case. Metaprogramming in tests is just as susceptable to mistakes as programming the domain. Yet, by instinct we always look there last, because we expect our tests to give us confidence, they should be correct. The example code is easy enough to follow, but most metaprogrammed tests contain more complexity, thus leading to even more fragile and fear instilling tests.

Also, if you find yourself wanting to defend metaprogrammed tests, ask yourself if you usually even provide as many clues as I have. Do you create test names that help you figure out what the problem was? Do you first get the letter and then compare it, or do you assert true and false, yielding even less information. If you don't give me at least as much information as I've given myself in my example, I can't even begin to imagine trying to find out what's wrong with a broken test.

The single largest problem with metaprogrammed tests is that they've unnecessarily added complexity to your test suite. This complexity reduces the maintainability of tests, ensuring that they are less likely to be maintained.

There is a better way.

You can approach the problem differently and still provide a concise solution. Looking at our issue another way, we simply want to test that certain values return A, B, C, D, or F. To me, that appears like I need 5 different tests, not 101. Here's what I consider to be a more maintainable solution.

class GradeTests < Test::Unit::TestCase
  def test_numbers_that_are_As
    assert_equal ["A"], (90..100).collect { |int| int.as_letter_grade }.uniq
  end

  def test_numbers_that_are_Bs
    assert_equal ["B"], (80..89).collect { |int| int.as_letter_grade }.uniq
  end
  
  # ... test the other letters
end

The above tests should be readable to anyone very quickly. They correctly provide the line number of a failing test when a test fails. Also, each test verifies only one piece of logic, greatly reducing complexity. Lastly, I can easily see in the test that it's written correctly, so any errors must be resulting from a mistake in the domain.

These tests instill more confidence and they are easier to digest and therefore maintain. These are tests that are more likely to live on and provide value. These are tests I thank my teammates for.

Update
Tammer Saleh correctly points out that the failure message for my last example would actually be worse than the failure message from the metaprogrammed tests. I was aware of that fact when I wrote up the entry, but I was unsure how to address the issue. If I were on a project I would write a custom assertion for expectations that would give me a descriptive error message while also allowing me to easily test what I want. That custom assertion would be well tested and could be designed to be general enough to apply across my entire test suite, thus infinitely more valuable than metaprogramming that only solves a problem for a specific test.

But, this isn't a project, it's an example. Still, I failed, I didn't give the complete answer. This is my attempt to resolve that situation. As I said, on a project I would use expectations, but for the purpose of this entry, I'll provide a custom assertion that could be easily used with test/unit.

The general solution is that I have an enumerable object and I want to verify the result of calling a method on each element of the enumerable. Thus, I should be able to create a general assertion that takes my expected single result, the enumerable, and the block that should be executed on each element. If all elements return actual results that match the expected value then the test passes. However, if any element does not return the expected value, then the expected value, the actual value, and the element are all described in the error message. The error message will contain all failures, not just the first one that fails.

Below is the code in full, but the following code would not be enough if this were a real project. Instead, if this were a real project this custom assertion should be tested with the same amount of effort that you put into testing any domain concept.

class Fixnum
  def as_letter_grade
    case self
      when 0..59 then "F"
      when 60..69 then "D"
      when 70..79 then "C"
      when 80..89 then "B"
      when 90..100 then "A"
    end
  end
end

require 'test/unit'

class GradeTests < Test::Unit::TestCase
  def test_numbers_that_are_As
    assert_enumerable_only_returns("A", 90..100) { |int| int.as_letter_grade }
  end

  def test_numbers_that_are_Bs
    assert_enumerable_only_returns("B", 80..89) { |int| int.as_letter_grade }
  end
  
  # ... test the other letters
end

class Test::Unit::TestCase
  def assert_enumerable_only_returns(expected, enumerable, &block)
    messages = enumerable.inject([]) do |result, element|
      actual = element.instance_eval(&block)
      result << "<#{expected}> expected but was <#{actual}> for #{element}" if expected != actual
      result
    end
    assert_block(messages.join("\n")) { messages.empty? }
  end
end

Additionally, here's the results from a failing test.

class GradeTests < Test::Unit::TestCase
  def test_numbers_that_are_Bs
    assert_enumerable_only_returns("B", 78..89) { |int| int.as_letter_grade }
  end
end

# >> Loaded suite -
# >> Started
# >> F
# >> Finished in 0.00063 seconds.
# >> 
# >>   1) Failure:
# >> test_numbers_that_are_Bs(GradeTests)
# >>     [-:28:in `assert_enumerable_only_returns'
# >>      -:17:in `test_numbers_that_are_Bs']:
# >> <B> expected but was <C> for 78
# >> <B> expected but was <C> for 79
# >> 
# >> 1 tests, 1 assertions, 1 failures, 0 errors

I would take this solution over any metaprogrammed solution I can think of.

Update 2
I decided to check out what the cyclomatic complexity would look like for defining tests in a loop compared to traditional definitions with custom assertions. I used Saikuro to give me cyclomatic complexity results.

Interestingly, the complexity of the looping test definition (8) is more than the complexity of the logic added to Fixnum (6). It's also double the complexity of the custom assertion version (4) of the tests. The custom assertion also registers a score of 4, but that doesn't concern me since I'll test the custom assertion.

For those interested in running the experiment the code I used can be found below. I defined a class method and called it explicitly because Saikuro reports complexity on a method basis, so I needed a method for it measure.

class Fixnum
  def as_letter_grade
    case self
      when 0..59 then "F"
      when 60..69 then "D"
      when 70..79 then "C"
      when 80..89 then "B"
      when 90..100 then "A"
    end
  end
end

require 'test/unit'

class LoopingGradeTests < Test::Unit::TestCase
  def self.define_tests
    (0..100).each do |index|
      letter = case index
        when 0..59 then "F"
        when 60..69 then "D"
        when 70..79 then "C"
        when 80..89 then "B"
        when 90..100 then "A"
      end
  
      define_method "test_#{index}_is_#{letter}" do
        assert_equal letter, index.as_letter_grade
      end
    end
  end
  define_tests
end

class CustomAssertionGradeTests < Test::Unit::TestCase
  def test_numbers_that_are_As
    assert_enumerable_only_returns("A", 90..100) { |int| int.as_letter_grade }
  end

  def test_numbers_that_are_Bs
    assert_enumerable_only_returns("B", 80..89) { |int| int.as_letter_grade }
  end
end

class Test::Unit::TestCase
  def assert_enumerable_only_returns(expected, enumerable, &block)
    messages = enumerable.inject([]) do |result, element|
      actual = element.instance_eval(&block)
      result << "<#{expected}> expected but was <#{actual}> for #{element}" if expected != actual
      result
    end
    assert_block(messages.join("\n")) { messages.empty? }
  end
end

Update 3
Since I ran Saikuro on the code, it only made sense to put it through Flog also.

The following code was flogged.

class LoopingGradeTests < Test::Unit::TestCase
  def self.define_tests
    (0..100).each do |index|
      letter = case index
        when 0..59 then "F"
        when 60..69 then "D"
        when 70..79 then "C"
        when 80..89 then "B"
        when 90..100 then "A"
      end
  
      define_method "test_#{index}_is_#{letter}" do
        assert_equal letter, index.as_letter_grade
      end
    end
  end
  define_tests
end

class CustomAssertionGradeTests < Test::Unit::TestCase
  def test_numbers_that_are_As
    assert_enumerable_only_returns("A", 90..100) { |int| int.as_letter_grade }
  end

  def test_numbers_that_are_Bs
    assert_enumerable_only_returns("B", 80..89) { |int| int.as_letter_grade }
  end
end

The flog score of the looping version was 15.3, the score of the custom assertion version was 6.5.

Both Saikuro and Flog marked the looping test definition with warnings and as a potential problem.