Comprehensive draft of Ruby code style to codify into Rubocop

Code of Code.md

Dev team standarts

Follow community and company code guidelines, facilitate collaboration

Adhere to Ruby Style Guide

Special mentions agreed to on 2018-07-20

1. Use no spaces around curly braces around Hash parentheses

# good
{one: 1, two: 2}

2. Use 'bad' example of spaces around interpolables

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

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

3. Indent conditional by one when assigning it ot a variable.

# bad
kind = case year  
       when 1930..1939 then 'Swing'
       when 1940..1950 then 'Bebop'
       else 'Jazz'
       end

# good (and a bit more width efficient)
kind =
  case year  
  when 1930..1939 then 'Swing'
  when 1940..1950 then 'Bebop'
  else 'Jazz'
  end

result =
  if some_cond
    calc_something
  else
    calc_something_else
  end

4. Use the 'leading dot' multi-line chaining style arguments for here

# bad
one.two.three.
  four

# good
one.two.three
  .four   

5. Align the parameters of a method call if they span more than one line.

# bad
Mailer.deliver(to: '[email protected]',
               from: '[email protected]',
               subject: 'Important message',
               body: source.text)

# good
Mailer.deliver(
  to: '[email protected]',
  from: '[email protected]',
  subject: 'Important message',
  body: source.text
) 

6. Use percent syntax for Regexps

# bad (requires escaping of forwardslashes, a common symbol in web-dev)
my_regexp = /\Ahttps?:\/\/(.*)\z/

# good
my_regexp = %r'\Ahttps?://(.*)\z'  

# too pro
my_regexp = %r'\A
  https? # scheme
  :// # delimiter
  (?<host_and_path>.*) # host and path
\z'x

# then
"http://example.com".match(my_regexp)[:host_and_path]  
#=> "example.com"

7. Prefer keyword args to positional args

# so 90s
def my_method(user, options={})  
end

# yay, 2010s
def my_method(user:, options: {})
end

8. Use double quoted strings

# bad because needs change if suddenly interpolation needed
name = 'Bozhidar'
interpolation = "I like #{fave}."

# good
name = "Bozhidar"
interpolation = "I like #{fave}."

# can use single if string will contain double quotes
sarcasm = 'I "like" it.'

9. Do not use parentheses if there are no arguments

# bad
def method_name()
end

method_name()

# good
def method_name
end

method_name

# or rarely
super()

10. When in doubt, use parentheses, especially if there are arguments

# bad
def method_name argument1, argument2

end

method_name argument1, argument2

# good
def method_name(argument1, argument2)
end

method_name(argument1, argument2)

11. Use the 'bad' example of no spaces around = operator in method definitions

# bad, but use this because it has superior readability.
def some_method(arg1=:default, arg2=nil, arg3=[])
end

# good, do not use this
def some_method(arg1 = :default, arg2 = nil, arg3 = [])
end

12. Use the 'bad' example and indent protected and private methods by one

# use the 'bad' example, indenting Protected and Private methods because, again, it's more readable. It's also "Rails" style.
class Person
  def public_method
  end

  protected

    def protected_method
    end

  private

    def private_method
    end   
end

13. Use return only as flow control

# bad, explicit, but verbose
def return_hash
  hash = {
    test: true
  }

  # binding.pry here, just before return line allows easy debugging.

  return hash
end

# good, uses implicit return of the last evaluated expression, terse
# debugging will require effort
def return_hash
  {test: true}  
end

14. Use return to terminate method execution quickly (i.e. use guard clauses)

# bad
def expensive_processing
  if should_not_preform_the_expensive_processing_on_this_record?
    nil
  else
    expensive_processing_result
  end
end

# good
def expensive_processing
  return nil if should_not_preform_the_expensive_processing_on_this_record?

  expensive_processing_result
end

15. Return nil when you only care about side-effects

# bad
def sideffects_only
  change_db_record1
  change_db_record2
  update_cache

  true
end

# good
def sideffects_only
  change_db_record1
  change_db_record2
  update_cache

  nil
end

16. Avoid self

# bad, Sometimes will just not work
def set_field_value(value)
  field = value
end

# bad, will work, but is ugly
def set_field_value(value)
  self.field = value
end

# good, avoids self and gotchas entirely
def set_field_value(value)
  assign_attributes(field: value)
end

17. Seperate guard cluases and return values from method body with newlines

# bad, pancake
def my_method
  return true if Config[:somekey]
  prepare_private_admin  
  admin = Admin.somescope.last  
  admin.auth_code
end

# good, newline after guard cluse and before return value
def my_method
  return true if Config[:somekey]

  prepare_private_admin  
  admin = Admin.somescope.last  
  
  admin.auth_code
end

Adhere to Rails Style Guide

Special mentions agreed to on 2018-07-20

1. Prefer if to unless in combination with appropriate predicate

# bad
unless user.present?
end

# good
if user.blank?
end

2. Split long scopes

# bad
scope :registered, -> { long.scope.code }

# good
scope :registered, -> {
  long.scope.code
}

3. Use named bind variables in SQL snippets

# ok
where("created_at < ?", 1.year.ago)

# bad
where("created_at < ? AND updated_at <= ?", 1.year.ago, 1.day.ago)

# good
where(
  "created_at < :wayback AND updated_at <= :recently",
  wayback: 1.year.ago, recently: 1.day.ago
)

4. Use simple service instance memoization

class SomeService
  attr_reader :user

  def initialize(user:)
    @user = user
  end

  # == bad ==
  def call    
    # some code that will get memoized  
  end

  memoize_method :call
  # =========

  # == good ==
  def call
    return @result if defined?(@result)

    intermediate = User.registered.find_by(name: "Bob")

    @result = intermediate&.last_name       
  end
  # ===========
end  

5. Prefer exists? to any? on query object presence

# BAD, will instantiate all records
user.applications.late.any?

# GOOD, performs a blazing fast SQL query only
user.applications.late.exists?

6. Do not use instance variables and always use explicit and full render calls
In controllers

# bad
def index
  @registration_containers
end

# good
def index
  registration_containers = ::Registration::Container.all

  render template: "admin/registration/containers/index", {registration_containers: registration_containers}
end

In views

# bad
= render 'admin/_partials/registration/containers/form'

# good
= render partial: 'admin/_partials/registration/containers/form', locals: {local_variable: value}

7. Write working database migrations

# bad, index: true just does not work!
add_column(:user, :favorite_number, :integer, null: false, default: 0, index: true) unless column_exists?(:user, favorite_number)

# good
add_column(:user, :favorite_number, :integer, null: false, default: 0) unless column_exists?(:user, favorite_number)
add_index(:user, :favorite_number) unless index_exists?(:user, :favorite_number)

8. Use a consistent tool for HTTP requests
RestClient seems good.

9. Prefer private to protected

# bad
protected

  def some_modularized_internal_method
  end

# good
private

  def some_modularized_internal_method
  end

10. Always specify a gem's version in Gemfile, prefer pessimistic locking

# specify a locked version of ruby for app to use, try not to change once deployment has happened
ruby "2.1.2"

# hard version for key gems that should not change ever
gem 'rails', '4.1.4'

# a safe way to allow gems to get patch-level bugfixes without risk of API changes
gem 'rails-assets-flag-icon-css', "~> 2.8.0" # will grow to 2.8.999, but never 2.9

# a risky way of specifying 'any version after N will do'.
# Bundler is smart enough to throttle super advanced versions if other gems (like Rails) depend on an older version
gem 'uglifier', '>= 1.3.0'

# if a gem comes from git, specify a commit ref or tag to use
gem 'rails-i18n', github: 'svenfuchs/rails-i18n', branch: 'master', ref: '6fe4bfa8f6f3' # For 4.x

11. Sort methods by scope and then alphabetically

Known scopes:

1. Class method, aka. singleton method, usually defined with def self.method_name,
2. Instance method, usually defined simply with def method_name,
3. Pure getter, a method with zero side-effects and state changes, only a return value
4. Setter, a method that changes state of at least one object. May have relevant return value (since Ruby has no void)
5. Private-only callback, a method, usually an instance setter, that is defined as private/protected, has at least one side-effect and is called by Rails model callbacks.
6. Class private/protected methods
7. Instance private/protected methods

Sort methods thusly:

1. Class methods, pure getters, alphabetically
2. Class methods, setters, alphabetically
3. Instance methods, pure getters, alphabetically
4. Instance methods, setters, alphabetically
5. Class private/protected methods
6. Private-only callbacks, alphabetically
7. Instance private/protected methods

Adhere to RSpec guidelines

1. Use an explicit, named subject for each method's describe block

describe "#befriend_people(*people)" do
  # bad
  let(:user) { create(:user, :with_friends, friend_count: 100) }

  let(:args) { ["Star1", "Star2"] }

  it "returns the number of friends" do
    expect(user.befriend_people(*args)).to eq(true)
  end
  
  # good
  subject(:befriending) { user.befriend_people(*args) }   
  
  let(:user) { create(:user, :with_friends, friend_count: 100) }

  let(:args) { ["Star1", "Star2"] }

  it { is_expected.to eq(true) }    
end  

2. Keep a consistent expectation format
2.1 Use spaces around subject block

# bad
subject(:eating_pizza){ user.eat_pizza }

# good
subject(:eating_pizza) { user.eat_pizza }

2.2 Do not put a space before expect and change blocks, shorter.

# bad
it "makes changes" do
  expect { eating_pizza }.to change { user.pizzas_eaten.size }.by(1)
end

# good
it "makes changes" do
  expect{ eating_pizza }.to change{ user.pizzas_eaten.size }.by(1)
end 

# good multiline with `and`
it "makes many changes and returns a value" do
  expect{ eating_pizza }.to(
    change{ user.pizzas_eaten.size }.by(1).
    and change{ Pizza.uneaten.size }.by(-1)
  )

  expect(eating_pizza).to be_a(Pizza)
end 

3. Prefer a single factory with traits to many factories

# bad
FactoryBot.define do
  factory :agreement do
    field { value }

    factory :agreement_expired do
      field { "expired" }
    end
  end 
end

create(:agreement_expired)

# good
FactoryBot.define do
  factory :agreement do
    field { value }

    trait :expired do
      field { "expired" }
    end
  end
end

create(:agreement, :expired)