blog comments 31 del.icio.us bookmarks 49 diggs 0 Google results 3

5.7
PostRank

Many Skinny Methods

From The Rails Way - all, 1 year ago, 10 views

This refactoring is based on a topic Marcel and I covered at RailsConf Europe.

Before

1
2
3
4
5
6
7
8
9
10
11
12
13
class Expense < ActiveRecord::Base
  belongs_to :payee
  protected
    
    # Nice and concise, but what happens as we add more rules
    # and how do we write test cases for the four different possible 
    # validation states?
    def validate
      errors.add("Not enough funds") if payee.balance - amount > 0
      errors.add("Charge is too great") if payee.account.maximum_allowable_charge > amount
    end
end

After

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
class Expense < ActiveRecord::Base
  belongs_to  :payee
  
  # Instead of one large validation method, break each individual
  # rule into methods, and declare them here.
  validate      :ensure_balance_is_sufficient_to_cover_amount
  validate      :amount_does_not_exceed_maximum_allowable_charge

  protected

    # These validation callbacks simply add error messages if a particular 
    # condition is met.   Each of them can be tested and understood on their own
    # without having to understand the entire body of the validate method.
    def ensure_balance_is_sufficient_to_cover_amount
      errors.add("Not enough funds") if insufficient_funds?
    end
    
    def amount_does_not_exceed_maximum_allowable_charge
      errors.add("Charge is too great") if exceeds_maximum_allowable_charge?
    end
    
    # By defining separate predicate methods we can test each of them individually
    # and new programmers can see the intent of the code, not just the implementation.


    # Instead of subtracting the amount from the balance and checking if the
    # value is greater than 0,  change the implementation to mirror the intent.
    # There was a bug in the before code,  this is more obvious.
    def insufficient_funds?
      amount > payee.balance
    end
    
    def exceeds_maximum_allowable_charge?
      payee.account.maximum_allowable_charge > amount
    end
end

While the refactored version may have more lines of code, but don’t let that scare you. It’s far more important for code to be human readable than incredibly concise.

comments

No comments yet.

You must be logged in to add your own comment.