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 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