The thing that complicates code more than anything else is conditional logic; “ifs’, "elses’, "unlesses’, nested and un-nested. I always try to eliminate them anyway I can because without them, the code is easier to understand and has a better rhythm. For instance:
def update_subscriptions
Subscription.find(:all).each do |each|
if each.expired?
each.renew!
else
each.update!
end
end
end
vs.
def update_subscriptions
subscriptions = Subscription.find :all
expired = subscriptions.select {|each| each.expired?}
expired.each {|each| each.renew!}
active = subscriptions.reject {|each| each.expired?}
active.each {|each| each.update!}
end
In the above code, we let Enumerable
‘s #select and #reject methods take care
of the conditional logic, i.e. we moved the conditional logic into Ruby and out
of our code. Look at how in the second example the code just flows up and down,
no indenting and very easy to read. Even non-programmers would choose the
second example as being more straightforward and elegant. Another example. Say
controller filters in Rails worked like this:
class EventsController < ApplicationController
def new
end
def create
end
def index
end
def show
end
private
def filter
if action_name == 'new' ||
action_name == 'create'
authenticate
end
end
# or a la Searching Literal
def filter
if %w(new create).include?(action_name)
authenticate
end
end
def authenticate
unless session[:user_id]
redirect_to login_url
end
end
end
In our #filter method we need conditional logic because we only want to require
the user to be logged in for the #new and #create actions. However, Rails’
filter class methods support an only
keyword parameter. So we can rewrite it
using the #before_filter class method instead:
class EventsController < ApplicationController
before_filter :authenticate, :only => [:new, create]
def new
end
def create
end
def index
end
def show
end
private
def authenticate
unless session[:user_id]
redirect_to login_url
end
end
end
Very nice. This time we moved the conditional logic into the framework. Now
somewhere in Rails’ #before_filter there’s an if
statement and 1 less if
in
our code. ActiveRecord::Base
validations also support this. Say that when
registering for our site, age is optional, but if you do provide an age, it has
to be between 18 and 30.
class User < ActiveRecord::Base
def validate
if ! age.blank?
if ! (18..30).include?(age)
errors.add :age, 'must be between 18 and 30'
end
end
end
end
I don’t like it, look at all the conditional logic. Fortunately
ActiveRecord::Base
‘s validation class methods can help us out:
class User < ActiveRecord::Base
validates_inclusion_of :age,
:allow_nil => true,
:in => 18..30,
:message => 'must be between 18 and 30'
end
Much better. Now Rails’ #validatesinclusionof has the 2 if
statements and
our code has 0.
Interestingly, Rails’ ActiveRecord::Base
callbacks do not support an if
parameter.
For example, say we have publications in our application. And we support 2 types of publications call them "ABC” and “XYZ”. Now the information for “ABC” and “XYZ” publications are provided by their respective sites, so we have to request the publication information from them via HTTP. We store it locally to avoid looking it up remotely every time we display a publication on our site. And that we don’t have to worry about the data getting out of sync because the remote data will never change.
class Publication < ActiveRecord::Base
def before_create
if type == 'ABC'
# the publication name is enough to uniquely identify itself
self.attributes = AbcGateway.find name
end
if type == 'XYZ'
# the publication name is enough to uniquely identify itself
self.attributes = XyzGateway.find name
end
end
end
More ugly conditionals. I wish Rails would support the following (using the
#before_create
class method instead of overriding the instance method):
class Publication < ActiveRecord::Base
before_create :abc, :if => Proc.new {|publication| publication.type == 'ABC'}
before_create :xyz, :if => Proc.new {|publication| publication.type == 'XYZ'}
def abc
self.attributes = AbcGateway.find name
end
def xyz
self.attributes = XyzGateway.find name
end
end
That’s much tighter without the conditionals.
One piece of Ruby syntactic sugar that I don’t care for are if
and unless
modifiers. My first example could be re-written more compactly using them like
so:
def update_subscriptions
Subscription.find(:all).each do |each|
each.renew! if each.expired?
each.update! unless each.expired?
end
end
I don’t like this style because the code now flows as if there’s no conditional
logic. The if
and unless
modifiers have hidden the obviousness of
conditional logic, i.e. the indenting. I like to see the conditional logic
because it makes the code look ugly and its always there reminding me to think
about how to refactor its complicatedness and ugliness out of there.
The same goes for the old ternary operator:
def update_subscriptions
Subscription.find(:all).each do |each|
each.expired? ? each.renew! : each.update!
end
end
Even shorter. But once again the conditional logic is not obvious. Also the double ‘?’ is ugly since Ruby allows ‘?’ in method names; this makes using the ternary operator not as elegant as in other languages.