Daily Refactor #74: Extract Method in Invoice

The Refactoring

While adding tests to my Redmine Invoice plugin, I found the following nasty bit of logic.

Before

1
2
3
4
5
6
7

class Invoice < ActiveRecord::Base
 
  def self.open
    invoices = self.find(:all)
    return invoices.select { |invoice| !invoice.fully_paid? && !invoice.late? }
  end
end

After

1
2
3
4
5
6
7
8
9
10
11
12
13

class Invoice < ActiveRecord::Base
 
  def self.open
    invoices = self.find(:all)
    return invoices.select { |invoice| invoice.open? }
  end
 
  # Is this invoice current but not fully paid?
  def open?
    !fully_paid? && !late?
  end
 
end

Review

By using extract method, I created the #open? predicate which made the code (and tests) easier to read. I need to work on the finder inside Invoice#open next, since it’s selecting every Invoice record and filtering in Ruby.

Reference commit