# bad - four spaces
def some_method
    do_something
end

# good
def some_method
  do_something
end

# bad
puts 'foobar'; # superfluous semicolon

# good
puts 'foobar'

# bad
puts 'foo'; puts 'bar' # two expressions on the same line

# good
puts 'foo'
puts 'bar'

puts 'foo', 'bar' # this applies to puts in particular

# bad
num.+ 42

# good
num + 42

# bad
sum=1+2
a,b=1,2
class FooError<StandardError;end

# good
sum = 1 + 2
a, b = 1, 2
class FooError < StandardError; end

# bad
e = M * c ** 2

# good
e = M * c**2

# bad
o_scale = 1 / 48r

# good
o_scale = 1/48r

# bad
foo &. bar
foo &.bar
foo&. bar

# good
foo&.bar

# bad
user&.address&.zip

#good
user && user.address.zip

#bad
user && user.address && user.address.zip

#good
class User
  def zip
    address&.zip
  end
end
user&.zip

# bad
some( arg ).other
[ 1, 2, 3 ].each{|e| puts e}

# good
some(arg).other
[1, 2, 3].each { |e| puts e }

# good - space after { and before }
{ one: 1, two: 2 }

# good - no space after { and before }
{one: 1, two: 2}

# bad
"From: #{ user.first_name }, #{ user.last_name }"

# good
"From: #{user.first_name}, #{user.last_name}"

# bad
! something

# good
!something

# bad
1 .. 3
'a' ... 'z'

# good
1..3
'a'...'z'

# bad
case
  when song.name == 'Misty'
    puts 'Not again!'
  when song.duration > 120
    puts 'Too long!'
  when Time.now.hour > 21
    puts "It's too late"
  else
    song.play
end

# good
case
when song.name == 'Misty'
  puts 'Not again!'
when song.duration > 120
  puts 'Too long!'
when Time.now.hour > 21
  puts "It's too late"
else
  song.play
end

# bad - pretty convoluted
kind = case year
when 1850..1889 then 'Blues'
when 1890..1909 then 'Ragtime'
when 1910..1929 then 'New Orleans Jazz'
when 1930..1939 then 'Swing'
when 1940..1950 then 'Bebop'
else 'Jazz'
end

result = if some_cond
  calc_something
else
  calc_something_else
end

# good - it's apparent what's going on
kind = case year
       when 1850..1889 then 'Blues'
       when 1890..1909 then 'Ragtime'
       when 1910..1929 then 'New Orleans Jazz'
       when 1930..1939 then 'Swing'
       when 1940..1950 then 'Bebop'
       else 'Jazz'
       end

result = if some_cond
           calc_something
         else
           calc_something_else
         end

# good (and a bit more width efficient)
kind =
  case year
  when 1850..1889 then 'Blues'
  when 1890..1909 then 'Ragtime'
  when 1910..1929 then 'New Orleans Jazz'
  when 1930..1939 then 'Swing'
  when 1940..1950 then 'Bebop'
  else 'Jazz'
  end

result =
  if some_cond
    calc_something
  else
    calc_something_else
  end

# bad
def some_method
  data = initialize(options)
  data.manipulate!
  data.result
end
def some_other_method
  result
end

# good
def some_method
  data = initialize(options)

  data.manipulate!

  data.result
end

def some_other_method
  result
end

# bad - It has two empty lines.
some_method


some_method

# good
some_method

some_method

# bad
class Foo
  attr_reader :foo
  def foo
    # do something...
  end
end

# good
class Foo
  attr_reader :foo

  def foo
    # do something...
  end
end

# bad
class Foo
  def bar; end
  private
  def baz; end
end

# good
class Foo
  def bar; end

  private

  def baz; end
end

# bad
class Foo

  def foo

    begin

      do_something do

        something

      end

    rescue

      something

    end

    true

  end

end

# good
class Foo
  def foo
    begin
      do_something do
        something
      end
    rescue
      something
    end
  end
end

# bad - easier to move/add/remove parameters, but still not preferred
some_method(
  size,
  count,
  color,
)

# bad
some_method(size, count, color, )

# good
some_method(size, count, color)

# bad
def some_method(arg1=:default, arg2=nil, arg3=[])
  # do something...
end

# good
def some_method(arg1 = :default, arg2 = nil, arg3 = [])
  # do something...
end

# bad (\ is not needed here)
result = 1 - \
         2

# bad (\ is required, but still ugly as hell)
result = 1 \
         - 2

# good
result = 1 -
         2

long_string = 'First part of the long string' \
              ' and second part of the long string'

# 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
user.set({ name: 'John', age: 45, permissions: { read: true } })

# good
user.set(name: 'John', age: 45, permissions: { read: true })

class Person < ActiveRecord::Base
  # bad
  validates(:name, { presence: true, length: { within: 1..10 } })

  # good
  validates :name, presence: true, length: { within: 1..10 }
end

# bad
puts (x + y)

# good
puts(x + y)

# bad
collection [index_or_key]

# good
collection[index_or_key]

# bad - single indent
menu_item = %w[Spam Spam Spam Spam Spam Spam Spam Spam
  Baked beans Spam Spam Spam Spam Spam]

# good
menu_item = %w[
  Spam Spam Spam Spam Spam Spam Spam Spam
  Baked beans Spam Spam Spam Spam Spam
]

# good
menu_item =
  %w[Spam Spam Spam Spam Spam Spam Spam Spam
     Baked beans Spam Spam Spam Spam Spam]

# bad - identifier is a Bulgarian word, using non-ascii (Cyrillic) characters
заплата = 1_000

# bad - identifier is a Bulgarian word, written with Latin letters (instead of Cyrillic)
zaplata = 1_000

# good
salary = 1_000

# bad
:'some symbol'
:SomeSymbol
:someSymbol

someVar = 5

def someMethod
  # some code
end

def SomeMethod
  # some code
end

# good
:some_symbol

some_var = 5

def some_method
  # some code
end

# bad
:some_sym_1

some_var_1 = 1

var_10 = 10

def some_method_1
  # some code
end

# good
:some_sym1

some_var1 = 1

var10 = 10

def some_method1
  # some code
end

# bad
class Someclass
  # some code
end

class Some_Class
  # some code
end

class SomeXml
  # some code
end

class XmlSomething
  # some code
end

# good
class SomeClass
  # some code
end

class SomeXML
  # some code
end

class XMLSomething
  # some code
end

# bad
SomeConst = 5

# good
SOME_CONST = 5

# bad
def even(value)
end

# good
def even?(value)
end

# bad
class Person
  def is_tall?
    true
  end

  def can_play_basketball?
    false
  end

  def does_like_candy?
    true
  end
end

# good
class Person
  def tall?
    true
  end

  def basketball_player?
    false
  end

  def likes_candy?
    true
  end
end

# bad - there is no matching 'safe' method
class Person
  def update!
  end
end

# good
class Person
  def update
  end
end

# good
class Person
  def update!
  end

  def update
  end
end

class Array
  def flatten_once!
    res = []

    each do |e|
      [*e].each { |f| res << f }
    end

    replace(res)
  end

  def flatten_once
    dup.flatten_once!
  end
end

# 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

# good
def +(other)
  # body omitted
end

# bad
def <<(other)
  @internal << other
end

# good
def <<(item)
  @internal << item
end

# bad
# Returns some string multiplied `other` times
def *(other)
  # body omitted
end

# good
# Returns some string multiplied `num` times
def *(num)
  # body omitted
end

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 block
elem # => NameError: undefined local variable or method `elem'

# bad
if some_condition then
  # body omitted
end

# bad
case foo
when bar then
  # body omitted
end

# bad
case expression
in pattern then
  # body omitted
end

# good
if some_condition
  # body omitted
end

# good
case foo
when bar
  # body omitted
end

# good
case expression
in pattern
  # body omitted
end

# bad
if
  some_condition
  do_something
  do_something_else
end

# good
if some_condition
  do_something
  do_something_else
end

# bad
result = if some_condition then something else something_else end

# good
result = some_condition ? something : something_else

# 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

# bad
result = if some_condition; something else something_else end

# good
result = some_condition ? something : something_else

# bad
if status == :active
  perform_action
elsif status == :inactive || status == :hibernating
  check_timeout
else
  final_action
end

# good
case status
when :active
  perform_action
when :inactive, :hibernating
  check_timeout
else
  final_action
end

# bad
if condition
  result = x
else
  result = y
end

# good
result =
  if condition
    x
  else
    y
  end

# bad
case expression
in pattern; do_something
end

# good
case expression
in pattern then do_something
end

# 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

# good
x = 'test'
if x
  # body omitted
end

# good
def named?
  !name.nil?
end

# good
def banned?
  !!banned_until&.future?
end

# good: and/or for control flow
x = extract_arguments or raise ArgumentError, "Not enough arguments!"
user.suspended? and return :denied

# bad
# and/or in conditions (their precedence is low, might produce unexpected result)
if got_needed_arguments and arguments_valid
  # ...body omitted
end
# in logical expression calculation
ok = got_needed_arguments and arguments_valid

# good
# &&/|| in conditions
if got_needed_arguments && arguments_valid
  # ...body omitted
end
# in logical expression calculation
ok = got_needed_arguments && arguments_valid

# bad
# &&/|| for control flow (can lead to very surprising results)
x = extract_arguments || raise(ArgumentError, "Not enough arguments!")

# bad
# Did author mean conditional return because `#log` could result in `nil`?
# ...or was it just to have a smart one-liner?
x = extract_arguments and log("extracted") and return

# good
# If the intention was conditional return
x = extract_arguments
if x
  return if log("extracted")
end
# If the intention was just "log, then return"
x = extract_arguments
if x
  log("extracted")
  return
end

# bad
if some_condition
  do_something
end

# good
do_something if some_condition

# another good option
some_condition && do_something

# 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

# bad
do_something if other_condition if some_condition

# good
do_something if some_condition && other_condition

# 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

# bad
unless success?
  puts 'failure'
else
  puts 'success'
end

# good
if success?
  puts 'success'
else
  puts 'failure'
end

# bad
if (x > 10)
  # body omitted
end

# good
if x > 10
  # body omitted
end

# 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

# bad
while some_condition
  do_something
end

# good
do_something while some_condition

# bad
do_something while !some_condition

# good
do_something until some_condition

# bad
while true
  do_something
end

until false
  do_something
end

# good
loop do
  do_something
end

# bad
begin
  puts val
  val += 1
end while val < 0

# good
loop do
  puts val
  val += 1
  break unless val < 0
end

# bad
def some_method(some_arr)
  return some_arr.size
end

# good
def some_method(some_arr)
  some_arr.size
end

# 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

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

# 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

# bad
END { puts 'Goodbye!' }

# good
at_exit { puts 'Goodbye!' }

# bad
def compute_thing(thing)
  if thing[:foo]
    update_with_bar(thing[:foo])
    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

# 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

# bad
fail SomeException, 'message'

# good
raise SomeException, 'message'

# bad
raise RuntimeError, 'message'

# good - signals a RuntimeError by default
raise 'message'

# bad
raise SomeException.new('message')
# Note that there is no way to do `raise SomeException.new('message'), backtrace`.

# good
raise SomeException, 'message'
# Consistent with `raise SomeException, 'message', backtrace`.

# bad
def foo
  raise
ensure
  return 'very bad idea'
end

# bad
def foo
  begin
    # main logic goes here
  rescue
    # failure handling goes here
  end
end

# good
def foo
  # main logic goes here
rescue
  # failure handling goes here
end

# bad
begin
  something_that_might_fail
rescue IOError
  # handle IOError
end

begin
  something_else_that_might_fail
rescue IOError
  # handle IOError
end

# good
def with_io_error_handling
  yield
rescue IOError
  # handle IOError
end

with_io_error_handling { something_that_might_fail }

with_io_error_handling { something_else_that_might_fail }

# bad
begin
  do_something # an exception occurs here
rescue SomeError
end

# good
begin
  do_something # an exception occurs here
rescue SomeError
  handle_exception
end

# good
begin
  do_something # an exception occurs here
rescue SomeError
  # Notes on why exception handling is not performed
end

# good
do_something rescue nil

# bad - this catches exceptions of StandardError class and its descendant classes
read_file rescue handle_error($!)

# good - this catches only the exceptions of Errno::ENOENT class and its descendant classes
def foo
  read_file
rescue Errno::ENOENT => e
  handle_error(e)
end

# bad
begin
  n / d
rescue ZeroDivisionError
  puts 'Cannot divide by 0!'
end

# good
if d.zero?
  puts 'Cannot divide by 0!'
else
  n / d
end

# bad
begin
  # calls to exit and kill signals will be caught (except kill -9)
  exit
rescue Exception
  puts "you didn't really want to exit, right?"
  # exception handling
end

# good
begin
  # a blind rescue rescues from StandardError, not Exception as many
  # programmers assume.
rescue => e
  # exception handling
end

# also good
begin
  # an exception occurs here
rescue StandardError => e
  # exception handling
end

# bad
begin
  # some code
rescue StandardError => e
  # some handling
rescue IOError => e
  # some handling that will never be executed
end

# good
begin
  # some code
rescue IOError => e
  # some handling
rescue StandardError => e
  # some handling
end

## text mode
# bad (only when reading from beginning to end - modes: 'r', 'rt', 'r+', 'r+t')
File.open(filename).read
File.open(filename, &:read)
File.open(filename) { |f| f.read }
File.open(filename) do |f|
  f.read
end
File.open(filename, 'r').read
File.open(filename, 'r', &:read)
File.open(filename, 'r') { |f| f.read }
File.open(filename, 'r') do |f|
  f.read
end

# good
File.read(filename)

## binary mode
# bad (only when reading from beginning to end - modes: 'rb', 'r+b')
File.open(filename, 'rb').read
File.open(filename, 'rb', &:read)
File.open(filename, 'rb') { |f| f.read }
File.open(filename, 'rb') do |f|
  f.read
end

# good
File.binread(filename)

## text mode
# bad (only truncating modes: 'w', 'wt', 'w+', 'w+t')
File.open(filename, 'w').write(content)
File.open(filename, 'w') { |f| f.write(content) }
File.open(filename, 'w') do |f|
  f.write(content)
end

# good
File.write(filename, content)

## binary mode
# bad (only truncating modes: 'wb', 'w+b')
File.open(filename, 'wb').write(content)
File.open(filename, 'wb') { |f| f.write(content) }
File.open(filename, 'wb') do |f|
  f.write(content)
end

# good
File.binwrite(filename, content)

f = File.open('testfile')
begin
  # .. process
rescue
  # .. handle error
ensure
  f.close if f
end

# bad - you need to close the file descriptor explicitly
f = File.open('testfile')
# some action on the file
f.close

# good - the file descriptor is closed automatically
File.open('testfile') do |f|
  # some action on the file
end

# bad - race condition with another process may result in an error in `mkdir`
unless Dir.exist?(path)
  FileUtils.mkdir(path)
end

# good - atomic and idempotent creation
FileUtils.mkdir_p(path)

# bad - race condition with another process may result in an error in `remove`
if File.exist?(path)
  FileUtils.remove(path)
end

# good - atomic and idempotent removal
FileUtils.rm_f(path)

# 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] # first => 1, list => [2, 3, 4]

hello_array = *'Hello' # => ["Hello"]

a = *(1..3) # => [1, 2, 3]

# bad
tmp = x
x = y
y = tmp

# good
x, y = y, x

# 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 are 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(',')

# 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

# bad
name = name ? name : 'Bozhidar'

# bad
name = 'Bozhidar' unless name

# good - set name to 'Bozhidar', only if it's nil or false
name ||= 'Bozhidar'

# 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

# bad
foo.object_id == bar.object_id

# good
foo.equal?(bar)

# bad
hash = {}
hash[foo.object_id] = :bar
if hash.key?(baz.object_id) # ...

# good
hash = {}.compare_by_identity
hash[foo] = :bar
if hash.key?(baz) # ...

# bad
Array === something
(1..100) === 7
/something/ === some_string

# good
something.is_a?(Array)
(1..100).include?(7)
some_string.match?(/something/)

# bad
something.kind_of?(Array)

# good
something.is_a?(Array)

# bad
something.instance_of?(Array)

# good
something.is_a?(Array)

# bad
var.class == Date
var.class.equal?(Date)
var.class.eql?(Date)
var.class.name == 'Date'

# good
var.instance_of?(Date)

# 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 Integer and Float 1

# bad
names.map { |name| name.upcase }

# good
names.map(&:upcase)

names = %w[Bozhidar Filipp 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)

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

# bad - easier to move/add/remove parameters, but still not preferred
[[1, 2, 3], [4, 5, 6]].each do |a, b, c,|
  a + b + c
end

# good
[[1, 2, 3], [4, 5, 6]].each do |a, b, c|
  a + b + c
end

# bad
[[1, 2, 3], [4, 5, 6]].each { |a, b, c,| a + b + c }

# good
[[1, 2, 3], [4, 5, 6]].each { |a, b, c| a + b + c }

# good - this comma is meaningful for array destructuring
[[1, 2, 3], [4, 5, 6]].map { |a,| a }

# 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

# 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

# bad
l = ->x, y { something(x, y) }

# good
l = ->(x, y) { something(x, y) }

# bad
l = ->() { something }

# good
l = -> { something }

# bad
p = Proc.new { |n| puts n }

# good
p = proc { |n| puts n }

# bad - looks similar to Enumeration access
l = ->(v) { puts v }
l[1]

# good - most compact form, but might be confusing for newcomers to Ruby
l = ->(v) { puts v }
l.(1)

# good - a bit verbose, but crystal clear
l = ->(v) { puts v }
l.call(1)

# bad
def some_method; end

# good
class SomeClass
  def some_method; end
end

# bad
def too_much; something; something_else; end

# okish - notice that the first ; is required
def no_braces_method; body end

# okish - notice that the second ; is optional
def no_braces_method; body; end

# okish - valid syntax, but no ; makes it kind of hard to read
def some_method() body end

# good
def some_method
  body
end

# good
def no_op; end

# bad
def fib(x) = if x < 2
  x
else
  fib(x - 1) + fib(x - 2)
end

# good
def the_answer = 42
def get_x = @x
def square(x) = x * x

# Not (so) good: has side effect
def set_x(x) = (@x = x)
def print_foo = puts("foo")

# bad
SomeClass::some_method
some_object::some_method

# good
SomeClass.some_method
some_object.some_method
SomeModule::SomeClass::SOME_CONST
SomeModule::SomeClass()

# bad
class Foo
  def self::some_method
  end
end

# good
class Foo
  def self.some_method
  end
end

# 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

# bad
x = Math.sin y
# good
x = Math.sin(y)

# bad
array.delete e
# good
array.delete(e)

# bad
temperance = Person.new 'Temperance', 30
# good
temperance = Person.new('Temperance', 30)

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

# bad
def some_method(foo: false, bar:, baz: 10)
  # body omitted
end

# good
def some_method(foo:, bar: false, baz: 10)
  # body omitted
end

# bad
def some_method(bar = false)
  puts bar
end

# bad - common hack before keyword args were introduced
def some_method(options = {})
  bar = options.fetch(:bar, false)
  puts bar
end

# good
def some_method(bar: false)
  puts bar
end

some_method            # => false
some_method(bar: true) # => true

# bad
def some_method(a, b = 5, c = 1)
  # body omitted
end

# good
def some_method(a, b: 5, c: 1)
  # body omitted
end

# bad
def some_method(options = {})
  bar = options.fetch(:bar, false)
  puts bar
end

# good
def some_method(bar: false)
  puts bar
end

# bad
def some_method(*args, &block)
  other_method(*args, &block)
end

# bad
def some_method(*args, **kwargs, &block)
  other_method(*args, **kwargs, &block)
end

# bad
# Please note that it can cause unexpected incompatible behavior
# because `...` forwards block also.
# https://github.com/rubocop/rubocop/issues/7549
def some_method(*args)
  other_method(*args)
end

# good
def some_method(...)
  other_method(...)
end

# bad
def some_method(&block)
  other_method(&block)
end

# good
def some_method(&)
  other_method(&)
end

class Person
  # extend/include/prepend go first
  extend SomeModule
  include AnotherModule
  prepend YetAnotherModule

  # inner classes
  class CustomError < StandardError
  end

  # constants are next
  SOME_CONSTANT = 20

  # afterwards we have attribute macros
  attr_reader :name

  # followed by other macros (if any)
  validates :name

  # public class methods are next in line
  def self.some_method
  end

  # initialization goes between class methods and other instance methods
  def initialize
  end

  # followed by other public instance methods
  def some_method
  end

  # protected and private methods are grouped near the end
  protected

  def some_protected_method
  end

  private

  def some_private_method
  end
end

# bad
class Person
  include Foo, Bar
end

# good
class Person
  # multiple mixins go in separate statements
  include Foo
  include Bar
end

# bad
FooError = Class.new(StandardError)

# okish
class FooError < StandardError; end

# ok
class FooError < StandardError
end

# bad

# foo.rb
class Foo
  class Bar
    # 30 methods inside
  end

  class Car
    # 20 methods inside
  end

  # 30 methods inside
end

# good

# foo.rb
class Foo
  # 30 methods inside
end

# foo/bar.rb
class Foo
  class Bar
    # 30 methods inside
  end
end

# foo/car.rb
class Foo
  class Car
    # 20 methods inside
  end
end

module Utilities
  class Queue
  end
end

# bad
class Utilities::Store
  Module.nesting # => [Utilities::Store]

  def initialize
    # Refers to the top level ::Queue class because Utilities isn't in the
    # current nesting chain.
    @queue = Queue.new
  end
end

# good
module Utilities
  class WaitingList
    Module.nesting # => [Utilities::WaitingList, Utilities]

    def initialize
      @queue = Queue.new # Refers to Utilities::Queue
    end
  end
end

# bad
class SomeClass
  def self.some_method
    # body omitted
  end

  def self.some_other_method
    # body omitted
  end
end

# good
module SomeModule
  module_function

  def some_method
    # body omitted
  end

  def some_other_method
    # body omitted
  end
end

# bad
module Utilities
  extend self

  def parse_something(string)
    # do stuff here
  end

  def other_utility_method(number, string)
    # do some more stuff
  end
end

# good
module Utilities
  module_function

  def parse_something(string)
    # do stuff here
  end

  def other_utility_method(number, string)
    # do some more stuff
  end
end

class Person
  attr_reader :first_name, :last_name

  def initialize(first_name, last_name)
    @first_name = first_name
    @last_name = last_name
  end

  def to_s
    "#{first_name} #{last_name}"
  end
end

# bad
class Person
  def initialize(first_name, last_name)
    @first_name = first_name
    @last_name = last_name
  end

  def first_name
    @first_name
  end

  def last_name
    @last_name
  end
end

# good
class Person
  attr_reader :first_name, :last_name

  def initialize(first_name, last_name)
    @first_name = first_name
    @last_name = last_name
  end
end

# bad
class Person
  def get_name
    "#{@first_name} #{@last_name}"
  end

  def set_name(name)
    @first_name, @last_name = name.split(' ')
  end
end

# good
class Person
  def name
    "#{@first_name} #{@last_name}"
  end

  def name=(name)
    @first_name, @last_name = name.split(' ')
  end
end

# bad - creates a single attribute accessor (deprecated in Ruby 1.9)
attr :something, true
attr :one, :two, :three # behaves as attr_reader

# good
attr_accessor :something
attr_reader :one, :two, :three

# good
class Person
  attr_accessor :first_name, :last_name

  def initialize(first_name, last_name)
    @first_name = first_name
    @last_name = last_name
  end
end

# better
Person = Struct.new(:first_name, :last_name) do
end

# bad
class Person < Struct.new(:first_name, :last_name)
end

# good
Person = Struct.new(:first_name, :last_name)

# bad
class Person < Data.define(:first_name, :last_name)
end

Person.ancestors
# => [Person, #<Class:0x0000000105abed88>, Data, Object, (...)]

# good
Person = Data.define(:first_name, :last_name)

Person.ancestors
# => [Person, Data, Object, (...)]

# bad
class Animal
  # abstract method
  def speak
  end
end

# extend superclass
class Duck < Animal
  def speak
    puts 'Quack! Quack'
  end
end

# extend superclass
class Dog < Animal
  def speak
    puts 'Bau! Bau!'
  end
end

# good
class Duck
  def speak
    puts 'Quack! Quack'
  end
end

class Dog
  def speak
    puts 'Bau! Bau!'
  end
end

class Parent
  @@class_var = 'parent'

  def self.print_class_var
    puts @@class_var
  end
end

class Child < Parent
  @@class_var = 'child'
end

Parent.print_class_var # => will print 'child'

# good
class SomeClass
  def public_method
    # some code
  end

  private

  def private_method
    # some code
  end

  def another_private_method
    # some code
  end
end

class TestClass
  # bad
  def TestClass.some_method
    # body omitted
  end

  # good
  def self.some_other_method
    # body omitted
  end

  # Also possible and convenient when you
  # have to define many class methods.
  class << self
    def first_method
      # body omitted
    end

    def second_method_etc
      # body omitted
    end
  end
end

class Westerner
  def first_name
    @names.first
  end

  alias given_name first_name
end

class Fugitive < Westerner
  def first_name
    'Nobody'
  end
end

class Fugitive < Westerner
  def first_name
    'Nobody'
  end

  alias given_name first_name
end

module Mononymous
  def self.included(other)
    other.class_eval { alias_method :full_name, :given_name }
  end
end

class Sting < Westerner
  include Mononymous
end

class TestClass
  # bad -- more work when class renamed/method moved
  def self.call(param1, param2)
    TestClass.new(param1).call(param2)
  end

  # bad -- more verbose than necessary
  def self.call(param1, param2)
    self.new(param1).call(param2)
  end

  # good
  def self.call(param1, param2)
    new(param1).call(param2)
  end

  # ...other methods...
end

# bad - FILES_TO_LINT is now defined globally
task :lint do
  FILES_TO_LINT = Dir['lib/*.rb']
  # ...
end

# good - files_to_lint is only defined inside the block
task :lint do
  files_to_lint = Dir['lib/*.rb']
  # ...
end

class Person
  def self.create(options_hash)
    # body omitted
  end
end

# bad
def initialize
  @x ||= 1
end

# good
def initialize
  @x = 1
end