Mark Needham

Thoughts on Software Development

Archive for September, 2010

RSpec: Another newbie mistake

with 3 comments

We recently had a spec which was checking that we didn’t receive a call to a specific method on an object…

describe "Our Object" do
	it "should not update property if user is not an admin" do
		our_user = Factory("user_with_role_x)
		User.stub!(:find).and_return(our_user)
 
		user.stub!(:is_admin?).and_return(false)
		user.should_not_receive(:property)
	end
end

…where ‘property’ refers to a field in the users table. In the code ‘property’ would get set like this:

class ObjectUnderTest
 
	def method_under_test
		user = User.find 4
 
		if user.is_admin?
			user.property = "random value"			
		end
	end

That test always passed but when we wrote the test to check that ‘property’ would be set if the user was an admin we found that it always failed.

The mistake is quite simple – the method that we wanted to check was received is actually called ‘property=’ rather than ‘property’!

Changing the spec to read like this solves the problem:

describe "Our Object" do
	it "should describe something" do
		our_user = Factory("user_with_role_x)
		User.stub!(:find).and_return(our_user)
 
		user.stub!(:is_admin?).and_return(false)
		user.should_not_receive(:property=)
	end
end

It didn’t take that long to figure it out but this is certainly a mistake you wouldn’t be able to make in a statically typed language.

Written by Mark Needham

September 30th, 2010 at 7:03 am

Posted in Ruby

Tagged with

Ruby: ActiveRecord 2.3.5 object equality

without comments

We learnt something interesting about the equality of ActiveRecord objects today while comparing two user objects – one which was being provided to our application by Warden and the other that we’d retrieved by a ‘User.find’ call.

Both objects referred to the same user in the database but were different instances in memory.

We needed to check that we were referring to the same user for one piece of functionality and were therefore able to make use of the ‘==’ method defined on ActiveRecord::Base which is defined in the documentation like so:

Public Instance methods
==(comparison_object)
Returns true if the comparison_object is the same object, or is of the same type and has the same id.

This worked fine but when trying to write a test around this code we found it quite difficult.

We wanted to simulate the fact that we had two objects in our application that referred to the same database entry but not the same object.

Our initial approach was something like this:

describe "ActiveRecord::Base Equality" do
	it "should let us have two objects which both point to the same user in the database" do
		# Create a user in the database with Factory Girl
		Factory(:user_with_x_role, :id => 15 )
 
		u = User.find 15
		u2 = User.find 15
 
		# the rest of our test where we'd insert u and u2 into the application
	end
end

Unfortunately when we ran the test we found that if we mutated ‘u’ in the production code then the value of ‘u2′ was also being mutated so they were somehow both referring to the same instance.

Our second attempt was to create a clone of the object so that they wouldn’t be referring to the same instance:

describe "ActiveRecord::Base Equality" do
	it "should let us have two objects which both point to the same user in the database" do
		# Create a user in the database with Factory Girl
		Factory(:user_with_x_role, :id => 15 )
 
		u = User.find 15
 
		u2 = u.clone
		u2.id = u.id
 
		# the rest of our test where we'd insert u and u2 into the application
	end
end

‘clone’ creates a copy of the object but removes the id. Since we want both our objects to have the same id so that they’ll be recognised as being equal we set the id on the next line.

Unfortunately we now found that the two objects weren’t being considered equal even though they were both of the same type and had the same id as per the documentation.

After a bit of binging with Google I came across an interesting post by fritz describing the code of ‘==’:

      # Returns true if the +comparison_object+ is the same object, or is of the same type and has the same id.
      def ==(comparison_object)
        comparison_object.equal?(self) ||
          (comparison_object.instance_of?(self.class) &&
            comparison_object.id == id &&
            !comparison_object.new_record?)
      end

In fact the ‘==’ code also takes into account whether or not the object passed to ‘==’ is new or not and since in our code we had ‘u == u2′ it would return false.

If we had ‘u2 == u’ then it would have passed!

Our current solution to the problem is to compare the ids of the objects in our code rather than the objects themselves but fritz also describes a way to monkey patch the ‘==’ method to make it a bit more useful.

Written by Mark Needham

September 30th, 2010 at 7:00 am

Posted in Ruby

Tagged with ,

Ruby: Intersection/Difference/Concatenation with collections

with one comment

We came across a couple of situations yesterday where we wanted to perform operations on two different arrays.

My immediate thought was that there should be some methods available similar to what we have in C# which Mike Wagg and I spoke about in our talk about using functional programming techniques in C#.

I was expecting to find methods with names indicating the operation they perform but in actual fact the methods are more like operators which makes for code that reads really well.

Intersection

This is useful when we have two collections and want to find the elements that exist in both of them.

In the world of C# we have an ‘Intersect’ method available as part of the LINQ library:

var collection1 = new int[] { 1,2,3,4 };
var collection2 = new int[] { 1,2,5,6 };
collection1.Intersect(collection2);

In Ruby we have the ‘&’ method:

ruby-1.8.7-p299 > [1,2,3,4]  & [1,2,5,6]
 => [1, 2]

Difference

This is useful when we have two collections and want to find items which are in one collection and not in the other.

In C# we can use the ‘Except’ method…

var collection1 = new int[] { 1,2,3,4 };
var collection2 = new int[] { 1,2 };
collection1.Except(collection2);

…whereas in Ruby we have the ‘-‘ method:

ruby-1.8.7-p299 > [1,2,3,4]  - [1,2]
 => [3, 4]

Concatenation

This is useful when we have two collections and want to join the two collections together.

In C# we’d use the ‘Union’ method..

var collection1 = new int[] { 1,2,3 };
var collection2 = new int[] { 4,5,6 };
collection1.Union(collection2);

..and in Ruby we have the ‘+’ method:

ruby-1.8.7-p299 > [1,2,3]  + [4,5,6]
 => [1, 2, 3, 4, 5, 6]

These methods and others on array are well documented on ruby-doc.org

Written by Mark Needham

September 29th, 2010 at 3:28 am

Posted in Ruby

Tagged with

FactoryGirl: ‘has_and_belongs_to_many’ associations and the ‘NoMethodError’

with one comment

We ran into a somewhat frustrating problem while using Factory Girl to create an object which had a ‘has_and_belongs_to_many’ association with another object.

The relevant code in the two classes was like this..

class Bar < ActiveRecord::Base
	has_and_belongs_to_many :foos, :class_name => "Foo", :join-table => "bar_foos"
end
class Foo < ActiveRecord::Base
	has_many :bars
end

…and we originally defined our ‘Bar’ factory like so:

Factory.define :bar do |f|
  f.association(:foos, :factory => :foo)
end
Factory.define :foo do |f|
   ...
end

On calling the following in our test

Factory(:bar)

we ended up with this stack trace:

NoMethodError in 'SomeController GET for some action'
undefined method `each' for #<Bar:0x102faabd8>
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/activerecord-2.3.5/lib/active_record/attribute_methods.rb:260:in `method_missing'
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/activerecord-2.3.5/lib/active_record/associations/association_collection.rb:320:in `replace'
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/activerecord-2.3.5/lib/active_record/associations.rb:1325:in `foos='
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/factory_girl-1.3.2/lib/factory_girl/proxy/build.rb:13:in `send'
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/factory_girl-1.3.2/lib/factory_girl/proxy/build.rb:13:in `set'
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/factory_girl-1.3.2/lib/factory_girl/proxy/build.rb:17:in `associate'
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/factory_girl-1.3.2/lib/factory_girl/attribute/association.rb:15:in `add_to'
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/factory_girl-1.3.2/lib/factory_girl/factory.rb:324:in `run'
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/factory_girl-1.3.2/lib/factory_girl/factory.rb:322:in `each'
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/factory_girl-1.3.2/lib/factory_girl/factory.rb:322:in `run'
/Users/mneedham/.rvm/gems/jruby-1.5.1/gems/factory_girl-1.3.2/lib/factory_girl/factory.rb:250:in `build'
/Users/mneedham/SandBox/ruby/some_controller_spec.rb:7:

The problem is that the Active Record code assumes that it will be passed an array when the ‘foo=’ method is called on the proxy ‘Bar’ object it creates. Unfortunately we’re passing a single ‘Foo’.

Instead we need to use the more verbose syntax and wrap the call to association in an array:

Factory.define :bar do |f|
  f.foos { |a| [a.association(:bar)] }
end

Dante Regis has written about this before but I found it sufficiently sufficiently frustrating that I thought I’d document it as well.

Written by Mark Needham

September 27th, 2010 at 2:18 pm

Posted in Ruby

Tagged with ,

RSpec: Fooled by stub!…with

with one comment

We had an RSpec spec setup roughly like this the other day…

describe "my stub test" do
  it "should be amazin" do
    Mark.stub!(:random).with("some_wrong_argument").and_return("something")
 
    Another.new.a_method
  end
end

…where ‘Mark’ and ‘Another’ were defined like so:

class Mark
  def self.random(params)
    "do some amazing stuff"
  end
end
class Another
  def a_method
    random = Mark.random("foo")
    # use random for something
  end
end

When we ran the spec we would get the following error message which was initially a little baffling:

NoMethodError in 'my stub test should be amazin'
undefined method `random' for Mark:Class
./rspec_spec.rb:9:in `a_method'
./rspec_spec.rb:17:

We spent quite a while looking at how we’d set up the spec to make sure we were actually calling ‘stub!’ on the right class before eventually realising that we had unintentionally added ‘with(“some_wrong_argument”)’ to that stub.

A little investigation of the RSpec code shows that this is the expected behaviour for this scenario:

module Spec
  module Mocks
    class Proxy
      ...
      def message_received(sym, *args, &block)
        expectation = find_matching_expectation(sym, *args)
        stub = find_matching_method_stub(sym, *args)
 
        if ok_to_invoke_stub?(stub, expectation)
          record_stub(stub, sym, args, &block)
        elsif expectation
          invoke_expectation(expectation, *args, &block)
        elsif expectation = find_almost_matching_expectation(sym, *args)
          record_almost_matching_expectation(expectation, sym, *args, &block)
        else
          @target.__send__ :method_missing, sym, *args, &block
        end
      end
    end
  end
end

In this case ‘find_matching_method_stub’ returns a nil value and since we didn’t set up any expectation on ‘Mark’ we fall through to the ‘method_missing’ exception.

Written by Mark Needham

September 26th, 2010 at 7:03 pm

Posted in Ruby

Tagged with ,

RSpec: Causing ourselves much pain through ‘attr’ misuse

with 2 comments

While testing some code that we were mixing into one of our controllers we made what I thought was an interesting mistake.

The module we wanted to test had some code a bit like this…

module OurModule
  def some_method
    @User = User.find(params[:id])
 
    # in the test code this is always true
    if @user == user
      ...
    end
  end
end

..and we had the spec setup like so:

describe 'OurController' do
  class TestController
    include OurModule
    attr_accessor :user
  end
 
  before(:each) do
    @controller = TestController.new
    @controller.user = Factory(:user)
  end	
 
  it "should do something" do
    # whole load of setup not related to our mistake
    @controller.some_method
  end
end

We created the ‘attr_accessor’ so that we would be able to choose a value for ‘user’ to simulate the fact that Warden mixes in a ‘user’ method into our ApplicationController at runtime.

The problem we ran into was that both ‘user’ and ‘@user’ inside ‘some_method’ were always returning the same value even though we set ‘user’ to be something else in our spec before each spec is run.

We eventually rotated pairs and immediately realised that the reason that was happening was because ‘user’ was in fact referring to the value we’d just set for ‘@user’ since in the test ‘user’ is created by an ‘attr_accessor’ call.

As it turns out Warden also mixes in a ‘current_user’ method so we changed the code to make use of that instead of ‘user’ since that was the team’s agreed standard and the call to ‘user’ hadn’t yet been replaced.

It does show an interesting side effect of setting instance variables in modules and also suggested that we had setup the test wrong to being with.

A better approach would have been to simulate what the actual code would do more closely and define a ‘user’ method which returned a canned user in a way that only relied on code in the test and wouldn’t change based on what the system under test did.

Written by Mark Needham

September 26th, 2010 at 6:57 pm

Posted in Ruby

Tagged with ,

Ruby: Control flow using ‘and’

with 5 comments

Something I’ve noticed while reading Ruby code is that quite frequently the flow of a program is controlled by the ‘chaining’ of different operations through use of the ‘and’ keyword.

I’ve noticed that this pattern is used in Javascript code as well and it’s particularly prevalent when we want to get a status for those operations after they’ve all been executed.

For example we might have the following code…

status = user.is_allowed_to_edit_foo? and user.update_foo(params[:foo]) and user.save

..where the user’s foo would only get updated and the record saved if they were actually allowed to edit their foo.

At the moment it seems quite strange to me because when I see operations chained together like that I assume that they’re all ‘query’ type operations but in Ruby it seems like ‘command’ type operations are used too.

To an extent the command query separation principle is being broken but it seems quite common to return a boolean value to indicate whether or not a state changing operation was successful.

I’d be more familiar with that type of code being written like this but I don’t think it reads as well:

status = false
if user.is_allowed_to_edit_foo?
	if user.update_foo(params[:foo])
		status = user.save
	end
end

While trying to think of a way of writing that code which I think would be more intention revealing I ended up with the following:

class User
  def is_allowed_to_edit_foo?(foo, &block)
    yield and return true if can_edit_foo?
    false
  end
 
  def update_foo(&block)
    #do some awesome stuff
    successful_update = true
    yield and return true if successful_update
    false
  end 
 
  def can_edit_foo?
    true
  end  
end
user = User.new
status = user.is_allowed_to_edit_foo?(params[:foo]) do
  user.update_foo do
     user.save
  end
end

I’m guessing the original code I posted is more idiomatic Ruby but I still think it’s interesting to see the different styles that you can write code in.

Written by Mark Needham

September 23rd, 2010 at 2:33 pm

Posted in Ruby

Tagged with

Ruby: Returning hashes using merge! and merge

with 7 comments

We came across an interesting problem today with some code which was unexpectedly returning nil.

The code that we had looked like this…

class SomeClass
	def our_method	
		a_hash = { :a => 2 }
		a_hash.merge!({:b => 3}) unless some_condition.nil?
	end
end

…and we didn’t notice the ‘unless’ statement on the end which meant that if ‘some_condition’ was nil then the return value of the method would be nil.

One way around it is to ensure that we explicitly return a_hash at the end of the method…

class SomeClass
	def our_method	
		a_hash = { :a => 2 }
		a_hash.merge!({:b => 3}) unless some_condition.nil?
		a_hash
	end
end

…but I think that looks a bit ugly.

Luckily Rails provides a method called ‘returning’ which I first learnt about from Reg Braithwaite’s blog post about the kestrel combinator.

That method is defined like so:

  def returning(value)
    yield(value)
    value
  end

And we can use it in our code like this:

class SomeClass
	def our_method	
		a_hash = { :a => 2 }
		returning a_hash do |h|
			h..merge!({:b => 3}) unless some_condition.nil?
		end
	end
end

Another way to return the merged hash without mutating the original would be to use the ‘merge’ method rather than ‘merge!':

class SomeClass
	def our_method	
		a_hash = { :a => 2 }
		a.hash.merge(!some_condition.nil? ? {:b => 3} : {})
	end
end

We could use that approach with ‘merge!’ as well but I’m not sure that it reads as nicely as the version which uses the ‘unless’ way.

Another approach that I started messing around with could be this…

class SomeClass
  def our_method
    a_hash = { :a => 2 }
    merge_unless(a_hash, {:b => 3}, proc { some_condition.nil? })
  end
end
 
def merge_unless(hash, other_hash, condition)
  if condition.call()
    hash
  else
    hash.merge(other_hash)
  end 
end

…although that’s probably a bit over the top seeing the collection of other ways we already have.

Written by Mark Needham

September 21st, 2010 at 8:24 pm

Posted in Ruby

Tagged with

Learning cycles at an overall project level

without comments

I was looking back over a post I wrote a couple of years ago where I described some learning cycles that I’d noticed myself going through with respect to code and although at the time I was thinking of those cycles in terms of code I think they are applicable at a project level as well.

The cycles I described were as follows:

  1. Don’t know what is good and what’s bad
  2. Learn what’s good and what’s bad but don’t know how to fix something that’s bad
  3. Learn how to make something that’s bad good

I think I’ve followed similar cycles with respect to how an overall project is run.

To start with I didn’t really know what it was that made a project run with an agile mindset different to anything I’d seen previously so I spent a lot of time observing the approaches my colleagues used, the processes they tried to drive and generally trying to understand what made a project tick.

Having worked on quite a few projects and seen similar underlying concepts working despite differing contexts I started to notice that the situations coming up were often the same or very similar to ones that I’d seen before.

At this stage I was more convinced that some of the approaches I’d already learnt could be useful but often deferred to more experienced colleagues or suggested improvements and relied on them to help drive them.

When I worked with Dermot he pointed out that the next step was to now be the one who can drive the changes that I want to see rather than relying on someone else to do it.

I’ve been trying to do that more recently and it’s not all that different to the way that I already try and influence the way that code is designed except it covers a wider spectrum of situations.

Books like ‘Fearless Change’ and ‘Agile Coaching‘ certainly have good tips for how to influence change but I find that more often than not I only really understand how to do something after I’ve made a complete mess of it the first time around.

I guess the downside of trying to influence situations more is that you put yourself in a position to be shot down so perseverance and knowing when to push and when to back off seem to be skills that will be particularly useful.

Written by Mark Needham

September 20th, 2010 at 6:56 pm

Posted in Learning

Tagged with ,

Rails: Faking a delete method with ‘form_for’

with 2 comments

We recently had a requirement to delete an item based on user input and wanting to adhere to the ‘RESTful’ approach that Rails encourages we therefore needed to fake a HTTP Delete method request.

The documentation talks a little about this:

The Rails framework encourages RESTful design of your applications, which means you’ll be making a lot of “PUT” and “DELETE” requests (besides “GET” and “POST”). However, most browsers don’t support methods other than “GET” and “POST” when it comes to submitting forms.

Rails works around this issue by emulating other methods over POST with a hidden input named “_method”, which is set to reflect the desired method:

The example provided is for the ‘form_tag’ helper method where you just need to pass a hash containing an entry with a key ‘:method’ and value ‘delete’.

We tried to do the same with ‘form_for’ but unfortunately found that that the hidden ‘_method’ field was still being set to ‘POST’.

It turns out that ‘form_for’ expects the ‘:method’ to be provided as part of the right hand most argument as part of a hash with the key ‘:html’.

We therefore need to write something like this if we want to simulate a delete request:

 <% form_for(item, :html => {:method => 'delete'}) do  %>
 ...
 <% end %>

The hidden field is eventually created inside form_tag_helper.rb:

def extra_tags_for_form(html_options)
          ...
          method_tag = case method
            when /^get$/i # must be case-insensitive, but can't use downcase as might be nil
              html_options["method"] = "get"
              ''
            when /^post$/i, "", nil
              html_options["method"] = "post"
              token_tag
            else
              html_options["method"] = "post"
              tag(:input, :type => "hidden", :name => "_method", :value => method) + token_tag
          end
 
         ...
end

I’m not finding the method signatures of many Ruby libraries particularly intuitive at the moment.

It seems like a lot of times the favoured approach is to pass in a hash into methods and then work off implicit knowledge that Ruby/Rails programmers have about the way that hashes are typically used.

While this makes methods very flexible it seems more difficult to understand how to use them as a consumer than if they took in specifically typed values as parameters.

It’ll be interesting to see if/how my opinion changes with respect to this.

Written by Mark Needham

September 20th, 2010 at 6:52 pm

Posted in Ruby

Tagged with ,