top | item 7429094

(no title)

feca | 12 years ago

This is actually a very complicated way of achieving the goal. It works, but it's not good from an economic point of view as it is more expensive in computational terms than a simpler OO alternative, which is to pass the size when you instantiate the MegaLotto::Drawing object.

For example, compare the proposed solution:

    MegaLotto.configure do |config|
      config.drawing_count = 10
    end

    MegaLotto::Drawing.new.draw
With the alternative:

    MegaLotto::Drawing.new(10).draw
If you want to make it extensible, you can use keyword arguments:

    MegaLotto::Drawing.new(size: 10).draw
The interface and the implementation are simpler, but also the performance is better because there are less method calls. If you look at the code of both implementations, you will find the simpler one easier to understand. As a side effect, you will also get simpler stack traces if anything goes wrong.

discuss

order

route66|12 years ago

In this case you are probably right. The solution with separate configuration shines though, when you have many values to configure (notwithstanding keyword args). But, and that is my main takeaway, TDD does apparently (and not surprisingly) not show the way to one or the other solution. On the contrary: the solution was already given in advance and the TDD part was more or less toying around with the moving parts of the solution.

soveran|12 years ago

It can be simplified even more:

    megalotto = MegaLotto.new
    megalotto.draw(10)

dennyabraham|12 years ago

This pattern is meant to enact a single, global configuration, so the extra allocations are not likely to affect performance noticeably.

danso|12 years ago

Right...but even in this trivial gem, there may be the need to configure more than one parameter. And by having Configuration be it's own object, you can encode some validation logic at the configuration stage.

I do agree that the constructor should have the option of passing in a Hash, which is then passed directly to the Configuration option.

feca|12 years ago

Another problem with the approach from the blog post is locality. If you need to draw 10 numbers in all places, then configuring that value in the module will work. But localy, when you look at the code, it won't tell you how many numbers you are drawing.

If you have this in many different places:

    MegaLotto::Drawing.new.draw
You don't have the information of how many numbers you are getting back. That's not a big deal, but adds to the cognitive load (or requires some comments). Also, if you need to draw different numbers in several different places, you will have to change the configuration many times:

    # First use, we need 10 numbers
    MegaLotto.configure do |config|
      config.drawing_count = 10
    end

    MegaLotto::Drawing.new.draw

    # Second use, we need 6 numbers
    MegaLotto.configure do |config|
      config.drawing_count = 6
    end

    MegaLotto::Drawing.new.draw
And as soon as you do that, you may need to take multi-threading into account, because you are mutating the class.

Extrapolating, it is like defining the size of an array:

    Array.new(4)
If instead you configure Array.new to have a given size for all instantiations, you also lose locality and you may run into thread safety issues.

In the case of MegaLotto::Drawing.new needing multiple configuration options, you can use keyword arguments. If you need too many arguments, maybe the abstraction is wrong. Even if you want to move forward with too many arguments, you can add getters/setters to the newly created instance:

   # Another approach which modifies the instance
   drawing = MegaLotto::Drawing.new
   drawing.size = 10
   drawing.draw #=> returns ten numbers
But this is not optimal design given the elements we have.

ballard|12 years ago

Yup. It's configuration management.

A setting maybe needed per app, per process, per thread, per class|module, per subclass|include/extended module, per instance or per method call. (Phew.) And that's probably only 90% of use-cases, not counting apps configured by something like a JSON api, ZooKeeper or Chef databags.

The trick is to isolate config from behavior out of code as much as possible, scala style. An obvious example is to use environment variables so apps can be reconfigured without touching code.