Tuesday, April 29, 2008

Software Development Lessons Learned From Poker

I recently wrote an article for InfoQ
I wasn't always a software developer. The two years before I joined ThoughtWorks I lived primarily off playing poker. Of course, if you've ever asked me about the tattoo on my forearm, you've already heard the story. If you haven't, feel free to ask me next time we get a drink together.

I've never regretted spending so much time playing poker. I believe it taught me quite a few lessons that apply widely to other topics. In fact the more I develop software the more I'm convinced that the two jobs are incredibly similar.

Learning
I approached learning poker the same way I approached learning software development: read as many books as possible. Over the course of 2 years I read every book on poker I could find. I stopped counting at 39. Of course, the same can be true for programming. I have 5 books in front of me right now that are queued up to read next, and I have a large collection of books that I've burned through in the past 3 years with ThoughtWorks.

I consider reading books, blogs, and magazines to be essential for both programming and playing poker, but in both professions reading books isn't good enough. You may be able to keep all the knowledge in your head, but knowing when to apply which rules is the mark of a true professional.

Reading material is great for learning, but it's almost always the context that determines when to apply a certain technique. Since books cannot be specific enough to provide all possible contexts, only experience can give you the ability to be able to make a quick decision that could end up costing you or your employer thousands or even millions of dollars.

...Read the entire article on InfoQ...



Monday, April 28, 2008

Understanding Why

Why is the most important question a software developer can ask. It is your responsibility as a developer and failing to do so is negligent and unprofessional.

I think Fred George would agree with me. Why is important enough to Fred that he uses it to determine if you are an apprentice or a journeyman.
An apprentice is a developer who is still learning the requisite skills... Techniques are being learned, but the why of the technique is still a bit vague. --Fred George
Unfortunately, I don't think why is asked nearly enough.

A great example is best practices. Best practices are defined by masters to help journeyman and apprentices learn and complete their tasks. But, best practices are rarely silver bullets, in fact the vast majority of best practices are heavily influenced by context. And, this is where the problem lies, the vast majority of software developers don't ask why a practice is considered best. Instead the common behavior is to blindly follow contextual advise without any thought given to the current situation.

Most test suites are also great examples of people failing to ask why. Programmers have gotten quite good at using OO concepts and patterns, but do they truly apply to testing? Largely, no. Of course, that's not surprising since tests are not object oriented.

Tests are often written using an object oriented language, but the tests themselves are not object oriented. In fact, the large majority of tests are purely procedural. Tests have one entry point and (hopefully) one exit point. Tests generally set up some state, then verify a result, that's it. Since tests are not object oriented, concepts such as DRY, inheritance, and polymorphism often lead to less readable code.

The problem is, rarely does anyone ask why a test is written and what purpose does it serve.

I write tests that help me write better applications. Tests must be readable, reliable, and performant. I want tests that focus on one thing, so when the test breaks I can quickly figure out what has gone wrong. I want tests that only break when something went wrong, so I don't waste time investigating side effects. And, I want tests that run quickly, so I can run them hundreds of times a day. When I write a test, I ask those questions, not whether my code follows good OO patterns.

Not asking why leads to poorly designed applications. Conversely, asking why often leads to innovation. Someone asking why led to C, Java, Smalltalk, Ruby, and every other language. Why also led to evolved frameworks and various other productivity boosting tools.

Asking why is almost always a good thing, and doing so is your professional responsibility.

Labels: , ,




Tuesday, April 15, 2008

Story 9: Display Customer Support Number

I thought it might be beneficial to create an entry that shows my workflow when implementing a few feature.

I generally work from stories, so it seemed logical to create an example story.
As a consumer
I would like to see the customer support number
So that I can call if I have a problem
There are a few technical details that make this story a bit more complicated than expected.Based on the story and the known technical details we come up with the following list of tasks.You may prefer more fine grained tasks (I usually do), but these should do for example purposes.

Task 1
The first thing I generally do is write a functional test. I want unit tests as well, and I'll probably write the unit tests and have them passing before I have the functional test passing, but starting with a functional test allows me to think at a higher level instead of getting bogged down in the details of the implementation. Therefore, the first test I write would probably look like this RSpec spec.

describe Configuration do
it "gets the support number from the client specific configuration" do
Configuration.for("localhost").support_number.should eql("212.646.9208")
end
end

[RSpec link]

Now, I know that this test isn't going to pass for awhile, but it gives me a good reminder of the direction that I want to continue to head in. At this point I would be looking to write a unit test that verifies that I can get the address from a stub configuration file. Having a unit test will allow me to get the Configuration class working without worrying about the configuration file. The unit test will also run significantly faster than the functional test, which benefits us in the long term.

Expectations do
expect "some url" do
File.stubs(:read).returns("localhost:\n configuration_url: some url")
Configuration.for("localhost")
end
end

[expectations link]

Ultimately I know that the Configuration::for method won't return the address, but my first task is to get the configuration file working, and this test is a good way to complete that task.

At this point I have failing tests, so I write the implementation of the Configuration class. Below is all of the code written at this point. (Of course, in a real application it would be broken out to appropriate files)

require 'yaml'
require 'rubygems'
require 'expectations'
require 'spec'

class Configuration < Struct.new(:support_number)
def self.for(environment)
YAML.load(File.read("config.yaml"))[environment]["configuration_url"]
end
end

Expectations do
expect "some url" do
File.stubs(:read).returns("localhost:\n configuration_url: some url")
Configuration.for("localhost")
end
end
# >> Expectations .
# >> Finished in 0.001 seconds
# >>
# >> Success: 1 fulfilled

describe Configuration do
it "gets the support number from the client specific configuration" do
Configuration.for("localhost").support_number.should eql("212.646.9208")
end
end
# >> F
# >>
# >> 1)
# >> NoMethodError in 'Configuration gets the support number from the client specific configuration'
# >> undefined method `support_number' for "127.0.0.1/config":String
# >>
# >> Finished in 0.008434 seconds
# >>
# >> 1 example, 1 failure

The unit test is passing; therefore, assuming that we've stubbed the configuration file correctly we've completed our first task.

Task 2
Our functional test is sufficient for verifying that task two is done correctly, so the next step is to write another unit test. For the purpose of the example, assume that there's a ConfigurationGateway::retrieve_from method that we can use to get the configuration from the affiliate web service. The configuration information is going to be returned by the gateway as a hash, so we'll create an appropriate stub so our unit tests don't rely on the real service. Our next unit test would look something like the following example.

Expectations do
expect "support number" do
File.stubs(:read).returns("localhost:\n configuration_url: some url")
ConfigurationGateway.stubs(:retrieve_from).returns(:support_number => "support number")
Configuration.for("localhost").support_number
end
end

A quick change to the Configuration class and we have our new unit test passing.

class Configuration < Struct.new(:support_number)
def self.for(environment)
configuration_url = YAML.load(File.read("config.yaml"))[environment]["configuration_url"]
self.new(ConfigurationGateway.retrieve_from(configuration_url)[:support_number])
end
end

Unfortunately, this change has broken our previously created unit test.

Expectations do
expect "some url" do
File.stubs(:read).returns("localhost:\n configuration_url: some url")
Configuration.for("localhost")
end
end
# >> Expectations F
# >> Finished in 0.001 seconds
# >>
# >> Failure: 1 failed, 0 errors, 0 fulfilled
# >>
# >> --Failures--
# >> -:40:in `expect'
# >> file <->
# >> line <40>
# >> expected: <"some url"> got: <#<struct Configuration support_number="212.646.9208">>

Our first unit test no longer works because we are no longer returning the address from the Configuration::for method. In fact, we don't need to expose the address outside of the Configuration class at all, so a behavior based unit test is probably a good choice for replacing our first unit test. The example below represents the new method for verifying that the configuration file is correctly returning the address.

Expectations do
expect ConfigurationGateway.to.receive(:retrieve_from).with("some url").returns({}) do
File.stubs(:read).returns("localhost:\n configuration_url: some url")
Configuration.for("localhost")
end
end

At this point, if the ConfigurationGateway is properly working and if your configuration file is in place, your functional test should also be passing.

Here's all the code we've written so far, including a fake ConfigurationGateway to show our Configuration class is working.

require 'yaml'
require 'rubygems'
require 'expectations'
require 'spec'

class Configuration < Struct.new(:support_number)
def self.for(environment)
configuration_url = YAML.load(File.read("config.yaml"))[environment]["configuration_url"]
self.new(ConfigurationGateway.retrieve_from(configuration_url)[:support_number])
end
end

class ConfigurationGateway
def self.retrieve_from(url)
# In actual code this would go to a external service...
{:support_number => "212.646.9208"}
end
end

Expectations do
expect ConfigurationGateway.to.receive(:retrieve_from).with("some url").returns({}) do
File.stubs(:read).returns("localhost:\n configuration_url: some url")
Configuration.for("localhost")
end

expect "support number" do
File.stubs(:read).returns("localhost:\n configuration_url: some url")
ConfigurationGateway.stubs(:retrieve_from).returns(:support_number => "support number")
Configuration.for("localhost").support_number
end
end
# >> Expectations ..
# >> Finished in 0.001 seconds
# >>
# >> Success: 2 fulfilled

describe Configuration do
it "gets the support number from the client specific configuration" do
Configuration.for("localhost").support_number.should eql("212.646.9208")
end
end
# >> .
# >>
# >> Finished in 0.008254 seconds
# >>
# >> 1 example, 0 failures

And, here's the configuration file that you'll also need to get the tests passing.

# config.yaml
localhost:
configuration_url: 127.0.0.1/config

At this point both our functional and unit tests are passing, and we are done with task two.

Task 3
For the purposes of this example, we're going to change the ConfigurationGateway to require the authentication token. If this were real code, I would expect the ConfigurationGateway to have this ability already built in, since we'd need a way to make the development server behave like production.

class ConfigurationGateway
def self.retrieve_from(url, token)
# In actual code this would go to a external service...
{:support_number => "212.646.9208"}
end
end

This change causes the functional test to break, but the unit tests do not break. That's OK. In order to get the functional tests passing we'll need to change the implementation, which will cause the unit tests to break. In the end, all of our tests will be updated to represent the correct interactions.

To make the functional tests pass we'll need to add a call to the AuthenticaionGateway to get the next authentication token. Following this change, the code below represents the new Configuration::for method.

class Configuration < Struct.new(:support_number)
def self.for(environment)
configuration_url = YAML.load(File.read("config.yaml"))[environment]["configuration_url"]
token = AuthenticationGateway.next_token
self.new(ConfigurationGateway.retrieve_from(configuration_url, token)[:support_number])
end
end

With this change in place the functional test now passes. To get it working locally, you'll need to add the fake AuthenticationGateway class that can be found below.

class AuthenticationGateway
def self.next_token
# In actual code this would go to an external service...
1979
end
end

At this point your functional test should be passing, but a unit test should be failing.

Expectations do
expect ConfigurationGateway.to.receive(:retrieve_from).with("some url").returns({}) do
File.stubs(:read).returns("localhost:\n configuration_url: some url")
Configuration.for("localhost")
end
end
# >> Expectations F.
# >> Finished in 0.00104 seconds
# >>
# >> Failure: 1 failed, 0 errors, 1 fulfilled
# >>
# >> --Failures--
# >> -:30:in `expect'
# >> file <->
# >> line <30>
# >> #<Mock:0x5168f0>.retrieve_from('some url', 1979) - expected calls: 0, actual calls: 1
# >> Similar expectations:
# >> #<Mock:0x5168f0>.retrieve_from('some url')

We need to update the ConfigurationGateway::for expectation, but we also need to remove the call to the external service from our unit tests. We do that by stubbing the AuthenticationGateway::next_token method.

Expectations do
expect ConfigurationGateway.to.receive(:retrieve_from).with("some url", "some token").returns({}) do
AuthenticationGateway.stubs(:next_token).returns("some token")
File.stubs(:read).returns("localhost:\n configuration_url: some url")
Configuration.for("localhost")
end

expect "support number" do
AuthenticationGateway.stubs(:next_token).returns("some token")
File.stubs(:read).returns("localhost:\n configuration_url: some url")
ConfigurationGateway.stubs(:retrieve_from).returns(:support_number => "support number")
Configuration.for("localhost").support_number
end
end

At this point we are done with task three. The code below represents all the code necessary for completing all 3 of our tasks.

require 'yaml'
require 'rubygems'
require 'expectations'
require 'spec'

class Configuration < Struct.new(:support_number)
def self.for(environment)
configuration_url = YAML.load(File.read("config.yaml"))[environment]["configuration_url"]
token = AuthenticationGateway.next_token
self.new(ConfigurationGateway.retrieve_from(configuration_url, token)[:support_number])
end
end

class ConfigurationGateway
def self.retrieve_from(url, token)
# In actual code this would go to a external service...
{:support_number => "212.646.9208"}
end
end

class AuthenticationGateway
def self.next_token
# In actual code this would go to an external service...
1979
end
end

Expectations do
expect ConfigurationGateway.to.receive(:retrieve_from).with("some url", "some token").returns({}) do
AuthenticationGateway.stubs(:next_token).returns("some token")
File.stubs(:read).returns("localhost:\n configuration_url: some url")
Configuration.for("localhost")
end

expect "support number" do
AuthenticationGateway.stubs(:next_token).returns("some token")
File.stubs(:read).returns("localhost:\n configuration_url: some url")
ConfigurationGateway.stubs(:retrieve_from).returns(:support_number => "support number")
Configuration.for("localhost").support_number
end
end

describe Configuration do
it "gets the support number from the client specific configuration" do
Configuration.for("localhost").support_number.should eql("212.646.9208")
end
end

Refactoring
We are done with our tasks, but we aren't ready to move on to the next card yet. Our functional test is great, but our unit tests specify way too much. They use stubs instead of mocks, which helps with creating more robust tests, but they suffer from High Implementation Specification.

The solution is to create smaller methods that are more test friendly.

The first refactoring is to change the configuration_url and token local variables to be method calls instead.

class Configuration < Struct.new(:support_number)
def self.for(environment)
self.new(ConfigurationGateway.retrieve_from(configuration_url(environment), authentication_token)[:support_number])
end

def self.configuration_url(environment)
YAML.load(File.read("config.yaml"))[environment]["configuration_url"]
end

def self.authentication_token
AuthenticationGateway.next_token
end
end

This change to the Configuration class makes our second unit test a bit more readable and robust.

Expectations do
expect "support number" do
Configuration.stubs(:authentication_token)
Configuration.stubs(:configuration_url)
ConfigurationGateway.stubs(:retrieve_from).returns(:support_number => "support number")
Configuration.for("localhost").support_number
end
end

At this point our second unit test looks pretty good, but the first one is still testing a bit too much. We can solve this by breaking the first test into 4 different tests. After breaking the first test up, our unit tests would look like the code below.

Expectations do
expect ConfigurationGateway.to.receive(:retrieve_from).with(nil, "token") do
Configuration.stubs(:authentication_token).returns("token")
Configuration.stubs(:configuration_url)
Configuration.configuration_results("localhost")
end

expect ConfigurationGateway.to.receive(:retrieve_from).with("configuration url", nil) do
Configuration.stubs(:authentication_token)
Configuration.stubs(:configuration_url).returns("configuration url")
Configuration.configuration_results("localhost")
end

expect AuthenticationGateway.to.receive(:next_token) do
Configuration.authentication_token
end

expect "some url" do
File.stubs(:read).returns("localhost:\n configuration_url: some url")
Configuration.configuration_url("localhost")
end

expect "support number" do
Configuration.stubs(:configuration_results).returns(:support_number => "support number")
Configuration.for("localhost").support_number
end
end

Whether or not to break the test apart would be a judgment call. The number of things I was specifying would drive me to go ahead and break it up, but if there had only been 5 values specified I might have left the test the way it was.

The Result
Below is all the code written for this entry. I hope you find it helpful.

require 'yaml'
require 'rubygems'
require 'expectations'
require 'spec'

class Configuration < Struct.new(:support_number)
def self.for(environment)
self.new(configuration_results(environment)[:support_number])
end

def self.configuration_results(environment)
ConfigurationGateway.retrieve_from(configuration_url(environment), authentication_token)
end

def self.configuration_url(environment)
YAML.load(File.read("config.yaml"))[environment]["configuration_url"]
end

def self.authentication_token
AuthenticationGateway.next_token
end
end

class ConfigurationGateway
def self.retrieve_from(url, token)
{:support_number => "212.646.9208"}
end
end

class AuthenticationGateway
def self.next_token
1979
end
end

Expectations do
expect ConfigurationGateway.to.receive(:retrieve_from).with(nil, "token") do
Configuration.stubs(:authentication_token).returns("token")
Configuration.stubs(:configuration_url)
Configuration.configuration_results("localhost")
end

expect ConfigurationGateway.to.receive(:retrieve_from).with("configuration url", nil) do
Configuration.stubs(:authentication_token)
Configuration.stubs(:configuration_url).returns("configuration url")
Configuration.configuration_results("localhost")
end

expect AuthenticationGateway.to.receive(:next_token) do
Configuration.authentication_token
end

expect "some url" do
File.stubs(:read).returns("localhost:\n configuration_url: some url")
Configuration.configuration_url("localhost")
end

expect "support number" do
Configuration.stubs(:configuration_results).returns(:support_number => "support number")
Configuration.for("localhost").support_number
end
end

describe Configuration do
it "gets the support number from the client specific configuration" do
Configuration.for("localhost").support_number.should eql("212.646.9208")
end
end

Labels: , , , ,




Wednesday, April 09, 2008

Splitting your Rails Test Suite

Before I began working primarily with Rails, I spent most of my time building C# applications. The majority of the C# projects that I was a part of had a unit test suite and a functional test suite.

The unit tests didn't interface with the filesystem, databases, or any external resource. The unit tests focused on testing one thing at a time, either isolated or with few collaborators (classes). They were designed to quickly give us feedback without using the full stack.

We also had functional tests that did test the full stack to ensure proper integration. As a result, we could run the unit tests to gain confidence that our code worked, and we could run the functional tests to ensure the code integrated correctly.

Things changed when I starting working with Rails. At first I was amazed by how quickly I could create websites, but I was disappointed by how slow writing tests became. I felt like I'd taken two steps forward and one step back. Rails had both unit and functional tests, but the definitions were different from the ones I'd grown used to in the Java and C# world. The unit tests felt more like model tests, while the functional tests felt more like controller tests. And, neither of them ran fast enough for me.

I decided to try splitting my Rails test suite based on the lessons I'd previously learned.

Speed
The original motivation for splitting my test suite was speed. I value feedback and I want it as quick as possible.

In 2006, I wrote about the idea of disconnecting the database for unit tests and the results. In 2007, Dan Manges rolled the concept into the UnitRecord gem. Then George Malamidis showed me that unit tests can run significantly faster if you don't load the Rails environment. I liked the concept of unit testing without Rails, but I knew we needed to provide a solution for unit testing ActiveRecord::Base subclasses. This is when arbs was born. The arbs gem is designed to make any ActiveRecord::Base subclass behave basically like a struct.

The result of using arbs was a unit test suite that ran in less than 1 second, including time to start and finish the process. When running tests in TextMate, the tests completed before the results window had finished drawing.

Positive Consequences
The original motivation for splitting our test suite was speed, but it resulted in a few other side benefits. A split test suite enabled us to write different tests based on what suite we were working with. For example, we use mocks and stubs in our unit test suite, but not in our functional test suite. Also, we use expectations to write unit tests and RSpec to write functional tests.

We use mocks in our unit tests to test in isolation. I prefer unit testing in isolation because it's a good way to mitigate cascading failures. However, by not allowing mocks in our functional test suite we ensure that the functional tests verify proper integration. We also use code coverage tools to ensure that everything is tested in isolation and tested while integrated. The result is robust tests that run quickly and verify integration.

Having two different suites also allows us to separate our testing concerns. Our unit tests have one assertion or one expectation per test. These fine grained unit tests focus on testing a single responsibility and generally do not break because of unrelated behavior changes. We find that the expectations unit testing framework allows us to focus on the essence of unit testing.

Our functional tests validate from a different perspective. They test at a much higher level and ensure that everything integrates as expected. Generally these types of tests need a description of why the test exists. Functional tests are generally also more resource intensive. We always strive to have one assertion per test, but due to resource requirements it's often necessary to have multiple assertions. We find that RSpec is the best tool for writing our functional tests.

Negative Consequences
Unfortunately, neither UnitRecord or arbs offer painless solutions. Splitting your test suite requires effort. Both UnitRecord and arbs rely on altering the behavior of core Rails classes. If you are unfamiliar with UnitRecord or arbs, you may see unexpected behavior when testing. Even though this is the case, I think the benefits outweigh the occasional confusion.

Having a split test suite can also cause confusion among developers. I've often worked with developers who wanted to write a new test but didn't know where the test belonged. I think "where should the test go" is the wrong question. Building comprehensive test suites requires that new functionality be tested but at the unit and functional level. Therefore, you should always write a unit test (if possible) and then ensure the logic is functionally tested. Since functional tests are generally course grained, it's often the case that the functionality will be covered by an existing test. If the functionality isn't covered by an existing test, then it makes sense to write a new functional test.

Conclusion
I prefer a split test suite because I value readable, reliable, and performant feedback. If you also value these things, you should give splitting your test suite a shot. Unfortunately, you'll be joining the minority. I believe that most Rails developers don't think it's worth the effort. Of course, at one point someone was a minority advocating for the same thing in Java, and now it's the norm.

Labels: , , , ,




Tuesday, April 08, 2008

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

Labels: , , ,




Monday, April 07, 2008

Alternatives for redefining methods

Ruby's open classes allow you define and redefine behavior pretty much at will; unfortunately, almost every option comes with caveats.

The example below is a gateway class that defines a process method. For the purposes of the example, assume that we need to redefine the process method on Gateway itself and call the original process method.* Also, assume that Gateway is not our class, so we cannot easily alter the original definition of process.

class Gateway
def process(document)
p "gateway processed document: #{document}"
end
end

Gateway.new.process("hello world")
# >> "gateway processed document: hello world"

Solution 1: alias

The following example uses alias to redefine the process method.

class Gateway
alias old_process process
def process(document)
p "do something else"
old_process(document)
end
end

Gateway.new.process("hello world")
# >> "do something else"
# >> "gateway processed document: hello world"

The example above creates an alias (old_process) for the process method. With an alias in place you can redefine the process method to anything you want and call the old_process method using the alias. This is probably the easiest solution and the most commonly used solution.

Unfortunately, it's not without problem. First of all, if someone redefines old_process you will get unexpected behavior. Second of all, the old_process method is really nothing more than an artifact of the fact that you have no other way to refer to the original method definition. Lastly, if the code is loaded twice, an infinite loop is created that causes the always painful to see 'stack level too deep' error.

Solution 2: alias_method_chain
Like I said, solution 1 is the most popular way to redefine a method. In fact, it's so popular Rails defines the alias_method_chain method to encapsulate the pattern. From the Rails source above alias_method_chain:
Encapsulates the common pattern of:
#
# alias_method :foo_without_feature, :foo
# alias_method :foo, :foo_with_feature
#
# With this, you simply do:
#
# alias_method_chain :foo, :feature
#
# And both aliases are set up for you.
#
# Query and bang methods (foo?, foo!) keep the same punctuation:
#
# alias_method_chain :foo?, :feature
#
# is equivalent to
#
# alias_method :foo_without_feature?, :foo?
# alias_method :foo?, :foo_with_feature?
#
# so you can safely chain foo, foo?, and foo! with the same feature.
Using alias_method_chain we can define our Gateway as the example below.

class Gateway
def process_with_logging(document)
p "do something else"
process_without_logging(document)
end
alias_method_chain :process, :logging
end

Gateway.new.process("hello world")
# >> "do something else"
# >> "gateway processed document: hello world"

Using alias_method_chain is nice because it's something familiar to many Rails developers. Unfortunately, it also suffers from the same problems as using alias on your own.

Solution 3: Close on an unbound method
The following code uses the class method "instance_method" to assign the "process" method (as an unbound method) to a local variable. The "process_method" local variable is in scope of the closure used to define the new process method, so it can be used within the process definition. Calling an unbound method is as simple as binding it to any instance of the class that it was unbound from and then using the call method.

class Gateway
process_method = instance_method(:process)
define_method :process do |document|
p "do something else"
process_method.bind(self).call(document)
end
end

Gateway.new.process("hello world")
# >> "do something else"
# >> "gateway processed document: hello world"

I've always preferred this solution because it doesn't rely on artifact methods that may or may not collide with other method definitions. Also, if the code is loaded multiple times the behavior is altered multiple times, but I find that easier to diagnose than when my only clue is "stack level too deep".

Unfortunately, this solution is not without flaws. Firstly, it relies on the fact that define_method uses a closure and has access to the unbound method. Of course this also implies that you have a handle on anything else defined in the same context. As with any closure, it's possible to accidentally create a memory leak. Also, (in MRI) I'm told that define_method takes 3 times as long to execute when compared to def.

Solution 4: Extend a module that redefines the method and uses super
This solution relies on creating a module with the new behavior and extending an instance with the module. Since the module is extended from the instance it will be checked first for the method definition when "process" is called (because it's the first ancestor). Since the module is the first ancestor it can use super to execute the process method defined in Gateway (the second ancestor).

module ProcessLogging
def process(document)
p "do something else"
super
end
end

Gateway.new.extend(ProcessLogging).instance_eval("class << self; self; end").ancestors
# => [ProcessLogging, Gateway, Object, Kernel]
Gateway.new.extend(ProcessLogging).process("hello world")
# >> "do something else"
# >> "gateway processed document: hello world"

This solution is my favorite because I can use def and super and never worry about creating any memory leaks or artifact methods.

Of course, it assumes that you get the opportunity to extend instances of the class. However, I haven't found that to be a problematic requirement.

* There are generally other options such as delegation, defining hooks, etc. Often I find these to be cleaner solutions and try that route first. But, sometimes redefining a method cannot be avoided.

Labels: , , ,




This page is powered by Blogger. Isn't yours?