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.
Share
Related posts:
