[rspec-users] Rate my code: refactoring from spec

Fernando Perez lists at ruby-forum.com
Wed Apr 15 12:36:12 EDT 2009


Hi,

I used to have the following method:

def Paypal.process_ipn(params)
  ...
  paypal = create!(params)
  paypal.order.update_from_ipn(completed?)
end

That method obviously is not easily specable because of the double dot
method call, and when specing it, it would hit the DB for nothing. I
used to actually spec the associated order status to know if everything
went on well which breaks the isolation of unit tests. Bad bad bad.

-----------

So I refactored it as following:

def Paypal.new_from_ipn(params)
  ...
  new(params)
end

So that it doesn't hit the DB when I spec the method, and specing the
method can now be done in isolation, as I only set some values returned
by IPN to something more compliant with my DB storage and then in the
controller that calls this method:

paypal = Paypal.new_from_ipn(params)
paypal.update_order if paypal.save!


So the paypal instance still gets saved, but instead of getting saved in
the model it occurs in the controller, no big deal. Also I added a new
instance method to Paypal class to abid to Demeter's law:

def update_order
  order.update_from_paypal_ipn!(completed?)
end

But now I am wondering how to spec this instance method, I thought of
the following:

it "should update the associated order" do
  @paypal = Paypal.new(:payment_status => 'Completed')
  @order = @paypal.order
  @order.expects(:update_from_paypal_ipn!).with(true)

  @paypal.update_order
end


Is that spec acceptable? I mean I am not sure about the following two
lines:

@order = @paypal.order
@order.expects(:update_from_paypal_ipn!).with(true)

I thought about this other solution which is slightly more complicated:
 @order = Order.new
  @paypal = Paypal.new(:payment_status => 'Completed')
  @paypal.stubs(:order).returns(@order)
  @order.expects(:update_from_paypal_ipn!).with(true)
  @paypal.update_order

So which is best?

And what do you think of my refactoring and my new specs? Did I improve
the code or is it just crap?
-- 
Posted via http://www.ruby-forum.com/.


More information about the rspec-users mailing list