Reputation: 13332
I always get great big lines of code at the top of my Rails models
. I am looking for suggestions for the best way to break them up with standard Ruby style. For example, one line I am looking at now is this:
delegate :occupation, :location, :picture_url, :homepage_url, :headline, :full_name, :to => :profile, :prefix => true, :allow_nil => true
What is the conventional style for breaking up these long method call lines?
Upvotes: 50
Views: 44910
Reputation: 160321
Something along the lines of:
delegate :occupation, :location, :picture_url,
:homepage_url, :headline, :full_name,
:to => :profile, :prefix => true, :allow_nil => true
Or if you like to highlight the option hash (a reasonable thing):
delegate :occupation, :location, :picture_url,
:homepage_url, :headline, :full_name,
:to => :profile, :prefix => true, :allow_nil => true
The idea of leaving that all on a single line strikes me as a craptastic idea, it means you have to scroll an arbitrary amount in order to see what's being delegated. Ew.
I'd probably line stuff up a bit, too, maybe alphabetize.
delegate :full_name, :headline, :homepage_url,
:location, :occupation, :picture_url,
:to => :profile, :prefix => true, :allow_nil => true
If the file didn't have much/any other substantive content, I might put each method symbol on its own line, just to make editing easier. In a larger file, I wouldn't want to take up the space for that.
Edit 2022: I’d probably put each symbol on its own line, including the options. In the long run, it’s just easier to deal with, and I’ve grown lazier.
Not that I ever think about this kind of stuff.
Edit I guess I do :/
These days I might group the delegated methods by "similarity", roughly:
delegate :full_name, :headline,
:location, :occupation,
:homepage_url, :picture_url,
to: :profile,
prefix: true,
allow_nil: true
My jury's hung on 1.9 hash syntax when the value is also a symbol; I think it looks funny. I'm also not sure where I'd indent it to–might lose it anyway during an IDE reformatting, but I kind of like how it looks above if I'm using the new syntax.
Upvotes: 22
Reputation: 12475
The short answer is it depends.
For a start you can save few chars using the "new" Ruby hash syntax:
result = very_long_method_name(something: 1, user: user, flange_factor: 1.34)
vs.
result = very_long_method_name(:something => 1, :user => user, :flange_factor => 1.34)
Sometimes you need to initialize an array or hash, especially for hashes it's nice to write them this way:
args = {
first_name: "Aldo",
email: "[email protected]",
age: Float::INFINITY
}
The same hash on the same line would be (not as nice):
args = {first_name: "Aldo", email: "[email protected]", age: Float::INFINITY}
Some methods require many params or these params have long names:
%table
%thead
%th
%td= t("first_name", scope: "activemodel.lazy_model.not_so_active_model", some_interpolation_argument: "Mr.", suffix: "(Jr.)")
In this case I would probably write it this way:
%table
%thead
%th
%td= t("first_name",
scope: "activemodel.lazy_model.not_so_active_model",
some_interpolation_argument: "Mr.",
suffix: "(Jr.)")
It's still not very beautiful but I guess is less ugly.
class person < ActiveRecord::Base
validates :n_cars, numericality: {
only_integer: true,
greater_than: 2,
odd: true,
message: t("greater_than_2_and_odd",
scope: "activerecord.errors.messages")
}
end
Again, not the most beautiful code on earth but It has some kind of structure.
Also, sometimes you could use variables to split lines. This is just an example, but basicly you name blocks of things (and sometimes after this you realise you can actually move that block in a method)
class person < ActiveRecord::Base
NUMERICALITY_OPTS = {
only_integer: true,
greater_than: 2,
odd: true,
message: t("greater_than_2_and_odd", scope: "activerecord.errors.messages")
}
validates :n_cars, numericality: NUMERICALITY_OPTS
end
Speaking of blocks (closures):
User.all.map { |user| user.method_name }
can be written like this:
User.all.map(&:method_name)
If you have proper blocks try to use do-end instead of curly braces:
nicotine_level = User.all.map do |user|
user.smoker? ? (user.age * 12.34) : 0.1234
end
Don't use the ternary if operator for complex things:
nicotine_level = user.smoker? ? (user.age * 1.234 + user.other_method) : ((user.age - 123 + user.flange_factor) * 0)
if user.smoker?
nicotine_level = user.age * 1.234 + user.other_method
else
nicotine_level = (user.age - 123 + user.flange_factor) * 0
end
If you have complex if statements like this:
if user.vegetarian? && !user.smoker? && (user.age < 25) && (user.n_girlfriends == 0) && (user.first_name =~ /(A|Z)[0-1]+/)
end
It's probably just better to move things in methods and make things not only shorter but also readable:
if user.healthy? && user.has_a_weird_name?
# Do something
end
# in User
def healthy?
vegetarian? && !smoker? && (age < 25) && (n_girlfriends == 0)
end
def user.has_a_weird_name?
user.first_name =~ /(A|Z)[0-1]+/
end
Heredoc is your friend...I always need to google in order to get the syntax right but once you get it right is makes certain things nicer to read:
execute <<-SQL
UPDATE people
SET smoker = 0
OK, this is a very bad example.
SQL
I tend to do this way for simple cases:
# Totally random example, it's just to give you an idea
def cars_older_than_n_days(days)
Car.select("cars.*, DATEDIFF(NOW(), release_date) AS age")
.joins(:brand)
.where(brand: {country: "German"})
.having("age > ?", days)
end
Sometimes queries are even worst. If I use squeel and the query is very big I tend to use parenthesis like this:
# Again, non-sense query
Person.where {
first_name = "Aldo" |
last_name = "McFlange" |
(
age = "18" &
first_name = "Mike" &
email =~ "%@hotmail.co.uk"
) |
(
person.n_girlfriends > 1 &
(
country = "Italy" |
salary > 1_234_567 |
very_beautiful = true |
(
whatever > 123 &
you_get_the_idea = true
)
)
)
}
I'd say, if possible try to avoid complex queries and split them in smaller scopes or whatever:
scope :healthy_users, lambda {
younger_than(25).
without_car.
non_smoking.
no_girlfriend
}
scope :younger_than, lambda { |age|
where("users.age < ?", age)
}
scope :without_car, lambda {
where(car_id: nil)
}
scope :non_smoking, lambda {
where(smoker: false)
}
scope :no_girlfriend, lambda {
where(n_girlfriends: 0)
}
This would be probably the best way.
Unfortunately people tend to write long lines and it's bad:
git diff
from the console having long lines is painfulI have a ruler in my editor so that I know when I'm about to cross the 80th char on the line. But rarely cross the line by few chars it's actually nicer than split it.
There are several ways of keep lines under the 80s and often depends on the situation. The problem with long lines is not just bad style, long lines are often a symptom of too much complexity.
Upvotes: 79
Reputation: 8313
Although the question already has two great answers I'd like to refer future readers to the Ruby Style Guide for such matters.
Currently the Source Code Layout section has plenty of information on how to break lines in various situations:
# starting point (line is too long)
def send_mail(source)
Mailer.deliver(to: '[email protected]', from: '[email protected]', subject: 'Important message', body: source.text)
end
# bad (double indent)
def send_mail(source)
Mailer.deliver(
to: '[email protected]',
from: '[email protected]',
subject: 'Important message',
body: source.text)
end
# good
def send_mail(source)
Mailer.deliver(to: '[email protected]',
from: '[email protected]',
subject: 'Important message',
body: source.text)
end
# good (normal indent)
def send_mail(source)
Mailer.deliver(
to: '[email protected]',
from: '[email protected]',
subject: 'Important message',
body: source.text
)
end
# bad - need to consult first line to understand second line
one.two.three.
four
# good - it's immediately clear what's going on the second line
one.two.three
.four
And it often turns out to be a «solution» for overly complex code as @Aldo already have mentioned:
# bad
some_condition ? (nested_condition ? nested_something : nested_something_else) : something_else
# good
if some_condition
nested_condition ? nested_something : nested_something_else
else
something_else
end
Upvotes: 7
Reputation: 3727
From my experience, it seems like the convention is actually not to break up the lines. Most projects I've seen, including the codebase of rails itself, seem to have no problem with having really long unbroken lines.
So I'd say if you want to follow convention, don't break up the lines. If you are determined to break the lines, then there's no widely followed convention for how to do it. You can use whichever coding style that you prefer.
Upvotes: 1