Exercises in Pruning Code, Episode #2

A story about building an application (including a brief discussion of functional programming)

Posted by 07/09/2008

I write Ruby on Rails applications for a living now. This is a pointless story about writing a section of one such application. I wrote this article for work. I've duplicated it here for no particular reason. It is slightly different here though - including a misguided discussion about functional programming that didn't go over very well at work.

I was recently working on a Ruby on Rails application that had a section for sending messages. This sounds pretty easy, right? I started with a User model and a Message model and some basic associations:

 class User < ActiveRecord::Base
    has_many :messages
 end

 class Message < ActiveRecord::Base
    belongs_to :user
 end  

Fig.1 - initial User and Message models

But when it came time to actually start building the application, I found this simple model code was not enough. The devil is in the details, as they say. There was a lot of functionality I needed to add beyond just a list of messages connected to a User.

1) Filtered views

I needed different views of the messages such as sent messages, drafts, and deleted messages.

How do I determine 'draft' status? Well one way is to fill in a delivered_at date whenever a message is sent. Then a draft is just a Message with no delivered_at date.

So after adding that field to the database I went to my messages_controller.rb file and added a few methods that looked sort of like this:

 def sent_mail
   @messages = user.messages.find(:all, 
     :conditions => ['delivered_at is not NULL'])
 end
 
 def drafts 
   @messages = user.messages.find(:all, 
     :conditions => ['delivered_at is NULL'])
 end

Fig.2 - initial fragment from messages_controller.rb

2) Pagination

Nobody wants to load a page of 1000 messages at a time, so I needed to be able to break up that list into limited sized chunks. I used the excellent plugin will_paginate for that purpose. Then my controllers methods got a little more verbose:

 def sent_mail
   @messages = user.messages.find(:all, 
      :conditions => ['delivered_at is not NULL']
    ).paginate(:page => (params[:page] == "" ? 1 : params[:page]))
 end
 
 def drafts 
   @messages = user.messages.find(:all, 
      :conditions => ['delivered_at is NULL']
    ).paginate(:page => (params[:page] == "" ? 1 : params[:page]))
 end

Fig.3 - fragment from messages_controller.rb with pagination

3) The ability to flag content (i.e. spam, objectionable content etc...)

What if someone gets spam in the message system - or something objectionable in some other way. Well I need to filter that stuff out. I added a Flag model and connected that to messages like so:

  class Message < ActiveRecord::Base
    has_many :flags
    belongs_to :user
  end

Fig.4 - Message model with flags added

However, at this point my controller methods are starting to look like this:

 def sent_mail
   @messages = user.messages.find(:all, 
       :conditions => ['delivered_at is not NULL and flags.flagged_item_id is NULL'], 
       :include => :flags
    ).paginate(:page => (params[:page] == "" ? 1 : params[:page]))
 end
 
 def drafts 
   @messages = user.messages.find(:all, 
       :conditions => ['delivered_at is NULL And flags.flagged_item_id is NULL'], 
       :include => :flags
    ).paginate(:page => (params[:page] == "" ? 1 : params[:page]))
 end

Fig.5 - fragment from messages_controller.rb with flags

I'm looking at some ugly code - with a lot of repetition. How do I pare this down?

Begin Pruning

My first thought is that if anything in my application can be flagged, I should be able to do a little meta-programming to create a find method that will give me only un-flagged items. Ideally I could even send in all the rest of the find arguments exactly the same.

There is the named_scope addition to Rails 2.x that does just that - but I also want something I can add to any class as a Mixin. That way I can write code like this:

  Message.unflagged_items.find(:all, :conditions => ['delivered_at is not NULL'])
  SomeOtherThing.unflagged_items.find(:all, :conditions => ...)

Fig.6 - call to imagined method unflagged_items

The method with_scope is a good candidate for sending in some pre-determined find conditions - but leaving it open to add more later. I'm wanting to add the following method to all my classes that need to be flagged:

  def unflagged_items(*args)  
    self.with_scope(:find => {:conditions => 'flags.flagged_item_id is NULL', 
        :include => :flags}) do  
      self.find(*args)
    end  
  end

Fig.7 - code for imaginary unflagged_items method

How do I do that? Well, I can turn that code into a Module and add it to any class automatically using a little metaprogramming:

  module Flaggable

    def self.included(base)
      base.class_eval do
        has_many :flags, :as => :flagged_item, :dependent => :destroy
      end
      base.extend(ClassMethods)
    end

    module ClassMethods

      def unflagged_items(*args)  
        self.with_scope(:find => {:conditions => 'flags.flagged_item_id is NULL', 
            :include => :flags}) do  
          self.find(*args)
        end  
      end  
    end

  end

Fig.8 - Flaggable module

After than any model I put the line include Flaggable in will have that method available. So if I include it in the User class I've added a method user.messages.unflagged_items which returns a sort of incomplete version of the find function - with all the necessary logic to limit the list to unflagged items already filled in. I still have to fill in the :all or :first or any other :conditions I want. But the function is sort of half-called. This is a useful thing - getting half-called functions. In functional programming it's called currying. I'll come back to that in a moment.

Anyway, So now my controller methods look like this:

 def sent_mail
    @messages = user.messages.unflagged_items(:all, 
       :conditions => ['delivered_at is not NULL']
    ).paginate(:page => (params[:page] == "" ? 1 : params[:page]))
 end
 
 def drafts 
   @messages = user.messages.unflagged_items(:all, 
       :conditions => ['delivered_at is NULL']
    ).paginate(:page => (params[:page] == "" ? 1 : params[:page]))
 end

Fig.9 - fragment from new messages_controller.rb

Continue Pruning

It's getting better, but isn't there some way I can pare it down even more? Now I'll go to the User model. Instead of simply using has_many :messages - since has_many supports blocks - I can add some more convenience methods to the User class:

  class User < ActiveRecord::Base

    has_many :sent_messages, :foreign_key => 'sender_id', 
          :class_name => 'Message' do
      def delivered_and_unflagged(page=1)
          unflagged_items(:all, :conditions => 'delivered_at IS NOT NULL'
        ).paginate(:page => page, :per_page => @messages_per_page) 
      end
    end

    has_many :draft_messages, :foreign_key => 'sender_id', 
          :class_name => 'Message', 
          :conditions => 'delivered_at IS NULL' do
      def paginated(page=1)
        paginate(:page => page, :per_page => @messages_per_page)
      end
    end

    def sent_mail(page=1)
      self.sent_messages.delivered_and_unflagged(page)
    end

    def drafts(page=1)
      self.draft_messages.paginated(page)
    end
  end

Fig.10 - more developed User model

I'm doing pretty well with reduction of code in my controller now. The only ugly bit of code leftover is the params[:page]... bit - but I can make that slightly better too. Now my controller code looks like this:

 def sent_mail
   @messages = user.sent_messages(params[:page] || 1)
 end
 
 def drafts 
   @messages = user.drafts(params[:page] || 1)
 end

Fig.11 - pruned fragment from messages_controller.rb

I'm happy enough with that. I've made different lists of messages for the currently logged on User that automatically paginate and filter out flagged items with just one line of code per method.

3) Next and Previous Message

I'm not done yet though - because the view page of a message needs a next and previous link. So if the user is looking at a draft - next should be the next draft - not the next sent message - and previous should be the previous draft - not the previous sent message. Make sense?

One way I could do this is to have a show_draft method, a show_sent_item method etc... and just call the correct link from the correct listing page (i.e. the list of all drafts page has links to show_draft, the sent items page has links to show_sent_item etc...).

There are 2 problems with this though. 1) That is creating several methods for basically one 'show' action. So they will all be virtually the same code over and over again. 2) I'm using a partial to render the list of messages - so I'd have to send in some way to create a different link based on the type of filter ('drafts', 'sent mail' etc...) but I'd rather just call render :partial => "message", :collection => @messages. I don't want the partial to have to worry about what particular filtered list of messages it happens to be rendering.

I'm sure there are a lot of ways to solve this. What I came up with was to add a 'from' value as a parameter for each link_to :action => 'show' in the partial. That way I could just append params[:action] to every url and by the time the controller gets the request, it knows where the request is coming from. This gives me the information I need to respond differently to the show action depending on that parameter. And leaves that logic out of the view.

This basically meant I needed to be able to identify and generate my list of messages based on the value of a string (i.e. params[:from]).

Brief aside concerning Functional Programming

The code I wrote at first looked something like this and was in the controller:

  def show
    @message = Message.find(params[:id])
    # need @messages for previous, next
    case params[:from]
    when 'sent_mail'
      @messages = user.sent_messages(params[:page] ...)
    when 'drafts'
      @messages = user.drafts(params[:page] ...)
    #...
  end

  def bulk_action
    # ... do bulk action

    # need @messages for previous, next
    case params[:from]
    when 'sent_mail'
      @messages = user.sent_messages(params[:page] ...)
    when 'drafts'
      @messages = user.drafts(params[:page] ...)
    #...
  end

Fig.12 - fragment of messages_controller.rb with new code

So I've lost some of my simplicity, I'm repeating myself again and my code is in need of pruning.

What I really need is a function that returns a function. Then I can call that from the bulk_action, show or any other method like drafts or sent_mail. It would be cool if I could do something like this in the User model:

 def get_messages_function(param)
   function = case param
     when 'sent_mail'
       self.sent_messages
     when 'drafts'
       self.drafts
   #...
 end

Fig.13 - imaginary code in messages_controller.rb

and then in my controller methods I could set a @messages instance with code like this:

  def sent_mail
    @messages = user.get_messages_function(params[:action])(param[:page] || 1)
  end
  
  def show
    @messages = user.get_messages_function(params[:from])(param[:page] || 1)
  end
  #...

Fig.14 - more imaginary code in messages_controller.rb code

So I need a function that returns a function waiting to receive arguments. This is similar to the with_scope method I mentioned earlier, and the idea of function currying. I need a function that's partially filled out - but not called yet - waiting for some parameters. You'd be surprised how often the need to do this comes up.

If you've ever heard the term 'first class function', being able to pass functions around like this is what that term is describing. It means a function can be passed around like any other object. A lot of languages have this. In Scheme, for instance, if I were trying to accomplish the same thing I would write code that looks something like this:

  (define (get-messages-function param)
     (cond
       ((equal? 'sent_mail param) sent_messages))
       ((equal? 'drafts param) drafts)))

Fig.15 - imaginary Scheme code

then I would set a messages variable like this:

  (define messages 
    (get-messages-function (params 'from) 
      (if (null? (params 'page)) (params 'page) 1)))

Fig.16 - imaginary Scheme code

Which, if you're not familiar with Scheme, is calling a get-messages-function and then, since that function returns a function, sending the arguments in automatically to the result of that call. It's very similar to my ideal code earlier.

In Python I would use an if statement because Python does not have a case statement, but it is conceptually similar to the Scheme example:

  def get_messages_function(param)
      if param == 'sent_mail':
          return self.sent_messages
      elif param == 'drafts':
          return self.drafts

Fig.17 - imaginary Python code

setting a messages variable like so:

  fn = user.get_messages_function(params['from'])
  messages = fn(params['page'] if params['page'] else 1)

Fig.18 - imaginary Python code

Both Scheme and Python have first-class functions, which is why I can call the code that way. Javascript is another functional language. In Javascript I can do something very similar:

  this.get_messages_function = function(param) {
    if(param == 'sent_mail') {
      return this.sent_messages;
    }
    else if(param == 'drafts') {
      return this.drafts;
    }
  }

Fig.19 - imaginary Javascript code

and then set a messages variable like this:

  var fn = user.get_messages_function(params['from']);
  var messages = fn('page' in params ? 1 : params['page']);

Fig.20 - imaginary Javascript code

Ruby has a lot of the features of functional languages such as Scheme, Python and Javascript, but in Ruby methods are not 'first-class' objects. In Ruby you cannot return a method and call it directly. You have to first convert it into a function object. This is what the lambda keyword is for (Proc.new would work just as well):

  def get_messages_function(param)
    x = case param.to_sym
        when :sent_mail
          lambda { |page| self.sent_mail(page) }
        when :drafts
          lambda { |page| self.drafts(page) }
    #    ...
  end

Fig.21 - fragment from User model

returns a function as an object waiting for the |page| argument. So I can put that code in my User class and I can call it like this in my controller:

 def sent_mail
  @messages = user.get_messages_function(params[:action]).call(param[:page] || 1)
 end
 
 def drafts 
  @messages = user.get_messages_function(params[:action]).call(param[:page] || 1)
 end

 def show
  @message = Message.find(params[:id])
  @messages = user.get_messages_function(params[:from]).call(param[:page] || 1)
 end

Fig.22 - fragment from new messages_controller.rb

Note, the only difference is I have to use call() instead of just ().

One last trick

I'm almost done. But I can go one step further in minimization of code. Taking advantages of the fact that a lambda can be converted to a block by putting an & in front of it. In the controller, since all the lambdas are taking that same params[:page] parameter - I can factor that out as a method accepting a block and do something like this:

  def sent_mail
    @messages = find_messages(&user.get_messages_function(:sent_mail))
  end

  def show
    @message = Message.find(params[:id])
    @messages = find_messages(&user.get_messages_function(params[:from])) 
  end

  private
  def find_messages(&func)
    yield(params[:page] || 1)
  end

Fig.23 - fragment from another revision to messages_controller.rb

It's odd looking, I admit. I've lost a little readability for the sake of density. But I've left myself very little code in the controller and nothing specific about controllers in the model. That much I like.

Conclusion

So if you ever writing a Ruby on Rails application that has messages that need to be filtered, paginated and include a detail view with a previous, next link - you might be able to glean some code from the article to help get started. Also, today's lesson is that it's sometimes handy to pass around functions as objects.

NOTE: I've included a zip file of various items related to this article. It includes some Ruby code as a demonstration which requires a sqlite3 installation. Also, I used Python to generate this document including all the color-coded sections. I've included that in case it is of interest to anyone. It requires the Mako and Pygments packages.

Comments

Post a comment

Your name:

Comment:


Total: 0.09 Python: 0.07 DB: 0.02 Queries: 32