Mark Needham

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

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.

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.

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.

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.

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.

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.

I thought it’d be interesting to write down some of my observations after working with Ruby and Rails for a couple more weeks so here are some more things I’ve come across and others that I’ve got confused with…

The :: operator

(apparently also known as the leading double colon operator)

I came across this while looking at some of the rails_warden code to try to understand how that gem opens the ActionController::Base class to add helper methods to it.

The code reads as follows:

77
78
79
80
81
82
83
84
85
86

 Rails.configuration.after_initialize do
    class ::ActionController::Base
      include RailsWarden::Mixins::HelperMethods
      include RailsWarden::Mixins::ControllerOnlyMethods
    end
 
    module ::ApplicationHelper
      include RailsWarden::Mixins::HelperMethods
    end
  end

The ‘::’ operator is used on line 78 and it means that Ruby will look in the global scope for the constant which follows the operator – in this case ActionController::Base. Marcos Ricardo explains this in more detail on his blog.

In this case I’m not entirely sure why the operator is necessary since there doesn’t seem to be another constant defined with the same name in the local scope.

The !! operator

This is another operator that I came across while reading the Warden code.

97
98
99

      def halted?
        !!@halted
      end

Sidu explained that this operator ensures that true or false will be returned rather than a truish/falish value.

For example:

ruby-1.8.7-p299 > !!nil
 => false

ruby-1.8.7-p299 > !!1
 => true

Single ‘=’ in if statements

A mistake I’ve made a few times now is using ‘=’ in if statements instead of ‘==’ which means that the code tends to fail in a somewhat confusing way.

I’m not sure if this is because I’ve been playing around with Clojure a bit recently and in Clojure you use ‘=’ for comparison or if I do this anyway and usually get saved by the compiler.

Either way it’s very frustrating!

Open classes

I’m used to being able to see exactly what is defined on a class in one place but in Ruby it’s possible to open a class from anywhere and add to it or change the existing behaviour.

I still don’t know all of the hooks that Rails provides for opening classes so it’s still a big magical for me at the moment.

As I mentioned in a post earlier in the week we’re using the declarative_authorization gem to control access to various parts of our application and as we’ve been migrating parts of the code base over to use that framework one thing we’ve noticed is that there seems to be a diminishing return in how much value we get from writing specs to cover each rule that we create.

We found that while it is possible to write a spec to cover every single rule it sometimes seems like the spec is just duplicating what the rule already describes.

This is especially the case if the rule just describes access to a particular controller action.

For example we had something similar to the following:

  role :x do
    has_permission_on :some, :to => [:action_1, :action_2, :action_3]
  end

For which we wrote specs similar to this (which make use of the test helpers provided with the framework):

context "user with role x" do
  before(:each) do
    @some_user = Factory.build(:user_with_role_x)
    Authorization.current_user=@some_user
  end
  context "some controller" do
    [:action_1, :action_2, :action_3].each do |action|
      it "should be allow to view #{action}" do
        should_be_allowed_to action, :some
      end
    end
  end
end

It certainly seems to me that if the code we’re writing uses a DSL then we lose the value of testing as a documentation feature and to an extent our tests are another slightly different DSL.

There is certainly still value in writing specs for more complicated rules where it is quite easy to write the code wrong but otherwise it seems like we’re just increasing the build time (albeit slightly) without necessarily getting much value.

Another approach that we’re using to test authorisation more indirectly is to ensure that we have automated tests at a higher level covering pieces of functionality which make use of the declarative authorization rules.

It doesn’t make sense to cover every single rule with this type of test because they take longer to run but I think there is still some value in ensuring that everything is ‘glued’ together correctly.

I got caught out for a little while today when comparing a value coming into a controller from ‘params’ which we were then comparing with a collection of numbers.

The code was roughly like this…

class SomeController
 
	def some_action
		some_collection = [1,2,3,4,5]
		selected_item = some_collection.find { |item| item == params[:id] }
	end
end

…and since the ‘id’ being passed in was ’1′ I was expected that we should have a selected item but we didn’t.

The actual code being executed would be…

[1,2,3,4,5].find { |item| item == "1" }

…which of course doesn’t match any values since we’re comparing a string to an integer.

The fix is relatively simple…

class SomeController
 
	def some_action
		some_collection = [1,2,3,4,5]
		selected_item = some_collection.find { |item| item == params[:id].to_i }
	end
end

…but I found it interesting that this is the type of thing that would have been caught if we had static type checking but is something you’d aim to cover with a test with a dynamic language.

In this case we had been refactoring the code and having slightly changed the way it worked internally our interaction tests didn’t quite cover this scenario properly so we had to go and update those.

So far I’ve certainly seen that Ruby/Rails can make you more productive and this is the first time I’ve seen a situation where I wished for C#’s static typing…if just for a little while.