Better Spree::Calculator

While working on improving the calculators for Spree I wanted to provide the developer implementing the calculators a proper message when they want to calculate something that is not supported with the calculator that they use.

The Spree::Calculator class now has a compute(computable) method that will call the specific compute methode based on the computable class. The exact implementation is shown further down.

I will illustrate how this works with an example. Let's create a sandbox store and add an empty calculator without any compute methods.

#app/models/my_dumb_calculator.rb
class MyDumbCalculator < Spree::Calculator  
end  

When that's done, let's add a unit test to make sure everything works.

require 'test_helper'

class MyDumbCalculatorTest < ActiveSupport::TestCase  
  test "compute with Spree::Order" do
    calculator = MyDumbCalculator.new
    assert_equal 10.0, calculator.compute(Spree::Order.new)
  end
end  

When we run the rake test:units command we receive the following output:

# Running tests:

E

Finished tests in 0.020577s, 48.5979 tests/s, 0.0000 assertions/s.

  1) Error:
MyDumbCalculatorTest#test_compute_with_Spree::Order:  
NotImplementedError: Please implement 'compute_order(order)' in your calculator: MyDumbCalculator  
    test/models/my_dumb_calculator_test.rb:6:in `block in <class:MyDumbCalculatorTest>'

1 tests, 0 assertions, 0 failures, 1 errors, 0 skips  

The relevant part is this:

NotImplementedError: Please implement 'compute_order(order)' in your calculator: MyDumbCalculator  

So to make the test pass all we have to do it add this to our MyDumbCalculator class:

def compute(order)  
  10.0
end  

Easypeasy :) But that's not what I found interesting.

Caller

I was planning on using the caller method to find the complete path of the class that called the compute method. However, that did not worked like I was expecting.

A class subclassing the Spree::Calculator without any methods does not gets added in the call stack. I was expecting that the subclass would be the caller since I thought it would call super.compute(computable) but when I ran the tests, it's the test class that is the actual caller.

Let's proof that, see the first implementation of the compute method below:

def compute(computable)  
  computable_name = computable.class.name.demodulize.underscore
  method = "compute_#{computable_name}".to_sym
  the_caller = caller[0].split(':')[0]
  begin
    self.send(method, computable)
  rescue NoMethodError
    raise NotImplementedError, "Please implement '#{method}(#{computable_name})' in your calculator: #{the_caller}"
  end
end  

So I save the the_caller variable with the first caller in the stack and get the path of the caller. Then I call the dynamic method and when the method is not present I rescue the NoMethodError and build the message to include the_caller

I does not work ofcourse, since I actually call the compute method on the subclassed calculator in the test. So the output of the message is:

NotImplementedError: Please implement 'compute_order(order)' in your calculator: /Users/peterberkenbosch/code/github/peterberkenbosch/spree/master/sandbox/test/models/my_dumb_calculator_test.rb  

And that is totally wrong obviously :) Lesson learned. So when subclassing it looks like the method of the super class is called from within the subclass itself I can use self for that purpose.

So to get the message I want I tried to find out if I can find out the path of a class. That was way to complex to even consider using in an exception message so I wend with the next best thing, the class name.

This is the current implementation of the compute(computable) method in Spree::Calculator

def compute(computable)  
  # Spree::LineItem -> :compute_line_item
  computable_name = computable.class.name.demodulize.underscore
  method = "compute_#{computable_name}".to_sym
  calculator_class = self.class
  begin
    self.send(method, computable)
  rescue NoMethodError
    raise NotImplementedError, "Please implement '#{method}(#{computable_name})' in your calculator: #{calculator_class.name}"
  end
end  

So when the method is not present I will raise a NotImplementedError that tells the developer exactly what method to implement and in what class that method should be.

Follow along on pull request #4041 and let me know what you think. Love your feedback.