02 - Syntax
Use
::
only to reference constants(this includes classes and modules) and constructors (likeArray()
orNokogiri::HTML()
). Do not use::
for regular method invocation.# bad SomeClass::some_method some_object::some_method # good SomeClass.some_method some_object.some_method SomeModule::SomeClass::SOME_CONST SomeModule::SomeClass()
Use
def
with parentheses when there are parameters. Omit the parentheses when the method doesn't accept any parameters.# bad def some_method() # body omitted end # good def some_method # body omitted end # bad def some_method_with_parameters param1, param2 # body omitted end # good def some_method_with_parameters(param1, param2) # body omitted end
Define optional arguments at the end of the list of arguments. Ruby has some unexpected results when calling methods that have optional arguments at the front of the list.
# bad def some_method(a = 1, b = 2, c, d) puts "#{a}, #{b}, #{c}, #{d}" end some_method('w', 'x') # => '1, 2, w, x' some_method('w', 'x', 'y') # => 'w, 2, x, y' some_method('w', 'x', 'y', 'z') # => 'w, x, y, z' # good def some_method(c, d, a = 1, b = 2) puts "#{a}, #{b}, #{c}, #{d}" end some_method('w', 'x') # => '1, 2, w, x' some_method('w', 'x', 'y') # => 'y, 2, w, x' some_method('w', 'x', 'y', 'z') # => 'y, z, w, x'
Avoid the use of parallel assignment for defining variables. Parallel assignment is allowed when it is the return of a method call, used with the splat operator, or when used to swap variable assignment. Parallel assignment is less readable than separate assignment.
# bad a, b, c, d = 'foo', 'bar', 'baz', 'foobar' # good a = 'foo' b = 'bar' c = 'baz' d = 'foobar' # good - swapping variable assignment # Swapping variable assignment is a special case because it will allow you to # swap the values that are assigned to each variable. a = 'foo' b = 'bar' a, b = b, a puts a # => 'bar' puts b # => 'foo' # good - method return def multi_return [1, 2] end first, second = multi_return # good - use with splat first, *list = [1, 2, 3, 4] hello_array = *'Hello' a = *(1..3)
Avoid the use of unnecessary trailing underscore variables during parallel assignment. Named underscore variables are to be preferred over underscore variables because of the context that they provide. Trailing underscore variables are necessary when there is a splat variable defined on the left side of the assignment, and the splat variable is not an underscore.
# bad foo = 'one,two,three,four,five' # Unnecessary assignment that does not provide useful information first, second, _ = foo.split(',') first, _, _ = foo.split(',') first, *_ = foo.split(',') # good foo = 'one,two,three,four,five' # The underscores is needed to show that you want all elements # except for the last number of underscore elements *beginning, _ = foo.split(',') *beginning, something, _ = foo.split(',') a, = foo.split(',') a, b, = foo.split(',') # Unnecessary assignment to an unused variable, but the assignment # provides us with useful information. first, _second = foo.split(',') first, _second, = foo.split(',') first, *_ending = foo.split(',')
Do not use
for
, unless you know exactly why. Most of the time iterators should be used instead.for
is implemented in terms ofeach
(so you're adding a level of indirection), but with a twist -for
doesn't introduce a new scope (unlikeeach
) and variables defined in its block will be visible outside it.arr = [1, 2, 3] # bad for elem in arr do puts elem end # note that elem is accessible outside of the for loop elem # => 3 # good arr.each { |elem| puts elem } # elem is not accessible outside each's block elem # => NameError: undefined local variable or method `elem'
Do not use
then
for multi-lineif/unless
.# bad if some_condition then # body omitted end # good if some_condition # body omitted end
Always put the condition on the same line as the
if
/unless
in a multi-line conditional.# bad if some_condition do_something do_something_else end # good if some_condition do_something do_something_else end
Favor the ternary operator(
?:
) overif/then/else/end
constructs. It's more common and obviously more concise.# bad result = if some_condition then something else something_else end # good result = some_condition ? something : something_else
Use one expression per branch in a ternary operator. This also means that ternary operators must not be nested. Prefer
if/else
constructs in these cases.# 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
Do not use
if x; ...
. Use the ternary operator instead.# bad result = if some_condition; something else something_else end # good result = some_condition ? something : something_else
Leverage the fact that
if
andcase
are expressions which return a result.# bad if condition result = x else result = y end # good result = if condition x else y end
Use
when x then ...
for one-line cases. The alternative syntaxwhen x: ...
has been removed as of Ruby 1.9.-
# bad - parentheses are required because of op precedence x = (not something) # good x = !something
-
# bad x = 'test' # obscure nil check if !!x # body omitted end x = false # double negation is useless on booleans !!x # => false # good x = 'test' unless x.nil? # body omitted end
The
and
andor
keywords are banned. It's just not worth it. Always use&&
and||
instead.# bad # boolean expression if some_condition and some_other_condition do_something end # control flow document.saved? or document.save! # good # boolean expression if some_condition && some_other_condition do_something end # control flow document.saved? || document.save!
Avoid multi-line
?:
(the ternary operator); useif/unless
instead.Favor modifier
if/unless
usage when you have a single-line body. Another good alternative is the usage of control flow&&/||
.# bad if some_condition do_something end # good do_something if some_condition # another good option some_condition && do_something
Avoid modifier
if/unless
usage at the end of a non-trivial multi-line block.# bad 10.times do # multi-line body omitted end if some_condition # good if some_condition 10.times do # multi-line body omitted end end
Avoid nested modifier
if/unless/while/until
usage. Favor&&/||
if appropriate.# bad do_something if other_condition if some_condition # good do_something if some_condition && other_condition
Favor
unless
overif
for negative conditions (or control flow||
).# bad do_something if !some_condition # bad do_something if not some_condition # good do_something unless some_condition # another good option some_condition || do_something
Do not use
unless
withelse
. Rewrite these with the positive case first.# bad unless success? puts 'failure' else puts 'success' end # good if success? puts 'success' else puts 'failure' end
Don't use parentheses around the condition of an
if/unless/while/until
.# bad if (x > 10) # body omitted end # good if x > 10 # body omitted end
Note that there is an exception to this rule, namely safe assignment in condition.
Do not use
while/until condition do
for multi-linewhile/until
.# bad while x > 5 do # body omitted end until x > 5 do # body omitted end # good while x > 5 # body omitted end until x > 5 # body omitted end
Favor modifier
while/until
usage when you have a single-line body.# bad while some_condition do_something end # good do_something while some_condition
Favor
until
overwhile
for negative conditions.# bad do_something while !some_condition # good do_something until some_condition
Use
Kernel#loop
instead ofwhile/until
when you need an infinite loop.# bad while true do_something end until false do_something end # good loop do do_something end
Use
Kernel#loop
withbreak
rather thanbegin/end/until
orbegin/end/while
for post-loop tests.# bad begin puts val val += 1 end while val < 0 # good loop do puts val val += 1 break unless val < 0 end
Omit parentheses around parameters for methods that are part of an internal DSL (e.g. Rake, Rails, RSpec), methods that have "keyword" status in Ruby (e.g.
attr_reader
,puts
) and attribute access methods. Use parentheses around the arguments of all other method invocations.class Person # bad attr_reader(:name, :age) # good attr_reader :name, :age # body omitted end # bad temperance = Person.new 'Temperance', 30 # good temperance = Person.new('Temperance', 30) # bad puts(temperance.age) # good puts temperance.age # bad x = Math.sin y # good x = Math.sin(y) # bad array.delete e # good array.delete(e) # bad expect(bowling.score).to eq 0 # good expect(bowling.score).to eq(0)
Omit the outer braces around an implicit options hash.
# bad user.set({ name: 'John', age: 45, permissions: { read: true } }) # good user.set(name: 'John', age: 45, permissions: { read: true })
Omit both the outer braces and parentheses for methods that are part of an internal DSL.
class Person < ActiveRecord::Base # bad validates(:name, { presence: true, length: { within: 1..10 } }) # good validates :name, presence: true, length: { within: 1..10 } end
Omit parentheses for method calls with no arguments.
# bad Kernel.exit!() 2.even?() fork() 'test'.upcase() # good Kernel.exit! 2.even? fork 'test'.upcase
Use the proc invocation shorthand when the invoked method is the only operation of a block.
# bad names.map { |name| name.upcase } # good names.map(&:upcase)
Prefer
{...}
overdo...end
for single-line blocks. Avoid using{...}
for multi-line blocks (multiline chaining is always ugly). Always usedo...end
for "control flow" and "method definitions" (e.g. in Rakefiles and certain DSLs). Avoiddo...end
when chaining.names = %w(Bozhidar Steve Sarah) # bad names.each do |name| puts name end # good names.each { |name| puts name } # bad names.select do |name| name.start_with?('S') end.map { |name| name.upcase } # good names.select { |name| name.start_with?('S') }.map(&:upcase)
Some will argue that multiline chaining would look OK with the use of {...}, but they should ask themselves - is this code really readable and can the blocks' contents be extracted into nifty methods?
Consider using explicit block argument to avoid writing block literal that just passes its arguments to another block. Beware of the performance impact, though, as the block gets converted to a Proc.
require 'tempfile' # bad def with_tmp_dir Dir.mktmpdir do |tmp_dir| Dir.chdir(tmp_dir) { |dir| yield dir } # block just passes arguments end end # good def with_tmp_dir(&block) Dir.mktmpdir do |tmp_dir| Dir.chdir(tmp_dir, &block) end end with_tmp_dir do |dir| puts "dir is accessible as a parameter and pwd is set: #{dir}" end
Avoid
return
where not required for flow of control.# bad def some_method(some_arr) return some_arr.size end # good def some_method(some_arr) some_arr.size end
Avoid
self
where not required. (It is only required when calling a self write accessor.)# bad def ready? if self.last_reviewed_at > self.last_updated_at self.worker.update(self.content, self.options) self.status = :in_progress end self.status == :verified end # good def ready? if last_reviewed_at > last_updated_at worker.update(content, options) self.status = :in_progress end status == :verified end
As a corollary, avoid shadowing methods with local variables unless they are both equivalent.
class Foo attr_accessor :options # ok def initialize(options) self.options = options # both options and self.options are equivalent here end # bad def do_something(options = {}) unless options[:when] == :later output(self.options[:message]) end end # good def do_something(params = {}) unless params[:when] == :later output(options[:message]) end end end
Don't use the return value of
=
(an assignment) in conditional expressions unless the assignment is wrapped in parentheses. This is a fairly popular idiom among Rubyists that's sometimes referred to as safe assignment in condition.# bad (+ a warning) if v = array.grep(/foo/) do_something(v) # some code end # good (MRI would still complain, but RuboCop won't) if (v = array.grep(/foo/)) do_something(v) # some code end # good v = array.grep(/foo/) if v do_something(v) # some code end
Use shorthand self assignment operators whenever applicable.
# bad x = x + y x = x * y x = x**y x = x / y x = x || y x = x && y # good x += y x *= y x **= y x /= y x ||= y x &&= y
Use
||=
to initialize variables only if they're not already initialized.# bad name = name ? name : 'Bozhidar' # bad name = 'Bozhidar' unless name # good - set name to 'Bozhidar', only if it's nil or false name ||= 'Bozhidar'
Don't use
||=
to initialize boolean variables. (Consider what would happen if the current value happened to befalse
.)# bad - would set enabled to true even if it was false enabled ||= true # good enabled = true if enabled.nil?
Use
&&=
to preprocess variables that may or may not exist. Using&&=
will change the value only if it exists, removing the need to check its existence withif
.# bad if something something = something.downcase end # bad something = something ? something.downcase : nil # ok something = something.downcase if something # good something = something && something.downcase # better something &&= something.downcase
Avoid explicit use of the case equality operator
===
. As its name implies it is meant to be used implicitly bycase
expressions and outside of them it yields some pretty confusing code.# bad Array === something (1..100) === 7 /something/ === some_string # good something.is_a?(Array) (1..100).include?(7) some_string =~ /something/
Do not use
eql?
when using==
will do. The stricter comparison semantics provided byeql?
are rarely needed in practice.# bad - eql? is the same as == for strings 'ruby'.eql? some_str # good 'ruby' == some_str 1.0.eql? x # eql? makes sense here if want to differentiate between Fixnum and Float 1
Avoid using Perl-style special variables (like
$:
,$;
, etc. ). They are quite cryptic and their use in anything but one-liner scripts is discouraged. Use the human-friendly aliases provided by theEnglish
library.# bad $:.unshift File.dirname(__FILE__) # good require 'English' $LOAD_PATH.unshift File.dirname(__FILE__)
Do not put a space between a method name and the opening parenthesis.
# bad f (3 + 2) + 1 # good f(3 + 2) + 1
If the first argument to a method begins with an open parenthesis, always use parentheses in the method invocation. For example, write
f((3 + 2) + 1)
.Always run the Ruby interpreter with the
-w
option so it will warn you if you forget either of the rules above!Do not use nested method definitions, use lambda instead. Nested method definitions actually produce methods in the same scope (e.g. class) as the outer method. Furthermore, the "nested method" will be redefined every time the method containing its definition is invoked.
# bad def foo(x) def bar(y) # body omitted end bar(x) end # good - the same as the previous, but no bar redefinition on every foo call def bar(y) # body omitted end def foo(x) bar(x) end # also good def foo(x) bar = ->(y) { ... } bar.call(x) end
Use the new lambda literal syntax for single line body blocks. Use the
lambda
method for multi-line blocks.# bad l = lambda { |a, b| a + b } l.call(1, 2) # correct, but looks extremely awkward l = ->(a, b) do tmp = a * 7 tmp * b / 50 end # good l = ->(a, b) { a + b } l.call(1, 2) l = lambda do |a, b| tmp = a * 7 tmp * b / 50 end
Don't omit the parameter parentheses when defining a stabby lambda with parameters.
# bad l = ->x, y { something(x, y) } # good l = ->(x, y) { something(x, y) }
Omit the parameter parentheses when defining a stabby lambda with no parameters.
# bad l = ->() { something } # good l = -> { something }
-
# bad p = Proc.new { |n| puts n } # good p = proc { |n| puts n }
Prefer
proc.call()
overproc[]
orproc.()
for both lambdas and procs.# bad - looks similar to Enumeration access l = ->(v) { puts v } l[1] # also bad - uncommon syntax l = ->(v) { puts v } l.(1) # good l = ->(v) { puts v } l.call(1)
Prefix with
_
unused block parameters and local variables. It's also acceptable to use just_
(although it's a bit less descriptive). This convention is recognized by the Ruby interpreter and tools like RuboCop and will suppress their unused variable warnings.# bad result = hash.map { |k, v| v + 1 } def something(x) unused_var, used_var = something_else(x) # some code end # good result = hash.map { |_k, v| v + 1 } def something(x) _unused_var, used_var = something_else(x) # some code end # good result = hash.map { |_, v| v + 1 } def something(x) _, used_var = something_else(x) # some code end
Use
$stdout/$stderr/$stdin
instead ofSTDOUT/STDERR/STDIN
.STDOUT/STDERR/STDIN
are constants, and while you can actually reassign (possibly to redirect some stream) constants in Ruby, you'll get an interpreter warning if you do so.Use
warn
instead of$stderr.puts
. Apart from being more concise and clear,warn
allows you to suppress warnings if you need to (by setting the warn level to 0 via-W0
).Favor the use of
sprintf
and its aliasformat
over the fairly crypticString#%
method.# bad '%d %d' % [20, 10] # => '20 10' # good sprintf('%d %d', 20, 10) # => '20 10' # good sprintf('%{first} %{second}', first: 20, second: 10) # => '20 10' format('%d %d', 20, 10) # => '20 10' # good format('%{first} %{second}', first: 20, second: 10) # => '20 10'
Favor the use of
Array#join
over the fairly crypticArray#*
with a string argument.# bad %w(one two three) * ', ' # => 'one, two, three' # good %w(one two three).join(', ') # => 'one, two, three'
Use
[*var]
orArray()
instead of explicitArray
check, when dealing with a variable you want to treat as an Array, but you're not certain it's an array.# bad paths = [paths] unless paths.is_a? Array paths.each { |path| do_something(path) } # good [*paths].each { |path| do_something(path) } # good (and a bit more readable) Array(paths).each { |path| do_something(path) }
Use ranges or
Comparable#between?
instead of complex comparison logic when possible.# bad do_something if x >= 1000 && x <= 2000 # good do_something if (1000..2000).include?(x) # good do_something if x.between?(1000, 2000)
Favor the use of predicate methods to explicit comparisons with
==
. Numeric comparisons are OK.# bad if x % 2 == 0 end if x % 2 == 1 end if x == nil end # good if x.even? end if x.odd? end if x.nil? end if x.zero? end if x == 0 end
Don't do explicit non-
nil
checks unless you're dealing with boolean values.# bad do_something if !something.nil? do_something if something != nil # good do_something if something # good - dealing with a boolean def value_set? !@some_boolean.nil? end
Do not use
END
blocks. UseKernel#at_exit
instead.# bad END { puts 'Goodbye!' } # good at_exit { puts 'Goodbye!' }
Avoid use of nested conditionals for flow of control.
Prefer a guard clause when you can assert invalid data. A guard clause is a conditional statement at the top of a function that bails out as soon as it can.
# bad def compute_thing(thing) if thing[:foo] update_with_bar(thing) if thing[:foo][:bar] partial_compute(thing) else re_compute(thing) end end end # good def compute_thing(thing) return unless thing[:foo] update_with_bar(thing[:foo]) return re_compute(thing) unless thing[:foo][:bar] partial_compute(thing) end
Prefer
next
in loops instead of conditional blocks.# bad [0, 1, 2, 3].each do |item| if item > 1 puts item end end # good [0, 1, 2, 3].each do |item| next unless item > 1 puts item end
Prefer
map
overcollect
,find
overdetect
,select
overfind_all
,reduce
overinject
andsize
overlength
. This is not a hard requirement; if the use of the alias enhances readability, it's ok to use it. The rhyming methods are inherited from Smalltalk and are not common in other programming languages. The reason the use ofselect
is encouraged overfind_all
is that it goes together nicely withreject
and its name is pretty self-explanatory.Don't use
count
as a substitute forsize
. ForEnumerable
objects other thanArray
it will iterate the entire collection in order to determine its size.# bad some_hash.count # good some_hash.size
Use
flat_map
instead ofmap
+flatten
. This does not apply for arrays with a depth greater than 2, i.e. ifusers.first.songs == ['a', ['b','c']]
, then usemap + flatten
rather thanflat_map
.flat_map
flattens the array by 1, whereasflatten
flattens it all the way.# bad all_songs = users.map(&:songs).flatten.uniq # good all_songs = users.flat_map(&:songs).uniq
Prefer
reverse_each
toreverse.each
because some classes thatinclude Enumerable
will provide an efficient implementation. Even in the worst case where a class does not provide a specialized implementation, the general implementation inherited fromEnumerable
will be at least as efficient as usingreverse.each
.# bad array.reverse.each { ... } # good array.reverse_each { ... }