Don't do this at home on Rails #1
- Languages: Ruby
- Difficulty: Easy
Intro
These series of articles will be dedicated to every day code, that I am working on. This could be the parts of my own projects or some ruby gems. Together, we will try to improve quality and readability of it.
Examples
#1 - avoid duplication
The first example is a scope, that fetches the records within a given range.
If the date param passed to this block responds to the first and last methods,
these are considered as start and end dates. Otherwise, it selects records for that date plus 1 day.
scope :at_date, lambda { |date|
if date.respond_to?(:last) && date.respond_to?(:first)
where("created_at >= ? AND created_at <= ?", date.first, date.last)
else
where("created_at >= ? AND created_at <= ?", date, date + 1.day)
end
}
What could you say about this code? Is it well written? This code has many flaws.
The first thing that caches the eye is duplicated where condition.
Just imagine, each time you want to change the query, you will need to update these 2 lines.
Lets fix this.
scope :at_date, lambda { |date|
if date.respond_to?(:last) && date.respond_to?(:first)
date_start = date.first
date_end = date.last
else
date_start = date
date_end = date_start + 1.day
end
where("created_at >= ? AND created_at <= ?", date_start, date_end)
}
Good, but that’s not all.
It seems to me very confusing, that date param could be
either Array or DateTime. Strictly speaking, it could be anything;
e.g String - it’s also responds to first/last methods, so as a result
we will get this query created_at >= 2 and created_at <= 3 for date = ‘2012-09-23’.
But wait, ActiveRecord’s query interface also supports ranges as an arguments,
so we could write something like this: where(created_at: date.first..date.last),
which will generate a query created_at BETWEEN <date.first> AND <date.last>.
scope :at_date, lambda { |range|
where(created_at: range)
}
#2 - try to search for existing method first
The second slice of code selects the channels, locked by the current user and free channels.
cu = current_user
locked = channels.select{ |ch| ch.is_locked_by?(cu) }
free = channels.select{ |ch| !ch.is_locked_by?(cu) }
Did you notice the reverse condition? Every time I see the code,
who looks like this, I thought, it should be already a method for this in ruby.
In fact, ruby and rails has a greater collection of methods. Take a look at ActiveSupport
methods http://apidock.com/rails/ActiveSupport. But what about our case? After a few minutes
of searching, I’ve found partition method (Doc),
which does exactly just we want to - splits collection into two arrays by a given condition.
locked, unlocked = channels.partition { |ch| ch.is_locked_by?(current_user) }
#3 - think about what you are writing right now
The third method is a simple method inside some model.
def has_description?
!self.description.blank?
end
I know what you are thinking right now - it’s not my :)
There are two drawbacks in the code above. The one significant is that there is already a
method for this in ActiveRecord. Rails creates a method called "{attribute}?",
which checks whether a field is defined or not. So we could remove has_description?
method with description?.
Note: you don’t have to use self inside the model methods, because we already in the context of an object.
And that’s all for today. Hope you’ve caught something for you!