Guides for programming in good, consistent style
- Commit small changes often instead of doing big commits every now and then
- Do not mix different issues in single commit
- If you spot any minor style issues while working on a feature commit them separately
- Always review changes before commiting to avoid accidental mistakes, and/or to avoid random files beeing added to the repository
- Do not rush your commits, review file changes and commit messages carefully before submitting
Capitalized, short (50 chars or less) summary
More detailed explanatory text, if necessary. Wrap it to about 72
characters.
- If there is a need to explain the changes further use bullet
points with dashes
- Wrap the points at 72 characters too
- Separate paragraphs with new line
- Do not use dots in the title or bullet points
- Write your commit message in the imperative (Fix bug, not Fixed
bug, Fixes the bug)
- Valid title should nicely end this sentence: "If submitted this
commit will: Fix bug"
- Do not repeat commit messages if you forget to include something.
Either squash two commits if the changes are not pushed or just
explicitly describe the second (missing) change.
- Always focus the message on the actual change, not the big
picture. Avoid "Setup base for new interface", use "Add routes
for ...".
http://project.management-system.com/ticket/123
Details:
- http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
- https://github.com/thoughtbot/guides/blob/main/git
-
Begin from updating a master branch:
$ git checkout master $ git pull origin master
-
Create a new branch for your new feature:
$ git checkout -b new_feature_branch # use snake_case for a branch name
-
Commit your changes often to allow earlier code review. After commiting very first changes, push your branch to remote and create a draft pull request:
$ git push origin new_feature_branch
-
Add a link to a newly created PR to the very end of a Trello ticket description.
-
When ready, change the status to 'Ready for review' near the bottom of your pull request to remove the draft state.
Do not add files related to local development environment to .gitignore
. It
should only contain entires common for everyone. If you want to exclude files
generated by your specific operating system or editor use .git/info/exclude
file instead.
Every newly created file should be created with a corresponding spec file. When staging changes for commit make sure that a staged file has a corresponding spec file (if applicable):
app/services/some/example/object.rb # file
spec/services/some/example/object_spec.rb # and its spec
In general, always write spec for your code. Preferably write specs before writing the implementation. If for some reason you cannot spec the current logic, then at least create pending spec file. Always include comment with explanation why the spec is missing.
RSpec.describe Some::Example::Object do
describe "#call" do
pending __FILE__
# FIXME I need help with this spec. I do not know how to test this
# implementation. It uses new framework and there is no examples
# for this kind of logic in the codebase yet.
end
end
When passing block to single spec in general use multiline do ... end
notation. Always use double quotes around spec title. Do not use
parenthesis after it
method name. Avoid condition in spec names.
If you need to describe what state is needed for given behaviour,
just wrap the text in context
block. Use this practice even if there
is no more branches for this example and no other specs. More often
than not, it makes sence to add another variant, where opposite
condition is described.
describe Contact do
context "without firstname" do
it "is invalid" do
...
end
end
end
In case of very trivial and repetable specs we do not use titles
at all. In such cases we use one line notation using it { ... }
syntax. Even though in general should
notaion is discouraged in the
newest versions of RSpec we still use it for such trivial examples. It
reads better and it is much shorter. In general cases we use one-line
notation for various types of Shoulda Matchers.
describe Contact do
it { should validate_presence_of(:firstname) }
it { should validate_uniqueness_of(:email) }
it { should validate_length_of(:password).is_at_least(10) }
end
If a spec one-liner exceeds 80 character revert back to a typical multi
line notation. Never omit titles in multiline notation. If the spec is
stil very trivial just use simple #method
notation.
describe "definitions" do
it "#kind" do
should define_enum_for(:kind).with(bad: 0, average: 1, good: 2, awesome: 3)
end
it "#historic_levels_build_by_day" do
should delegate_method(:build_by_day).to(:historic_levels).with_prefix
end
it "#amount" do
# When breaking a chain of methods do so for each dot '.'
expect(model)
.to have_db_column(:uid)
.of_type(:string)
.with_options(null: false)
end
end
Always put one empty line between two do end
blocks. Do not place
space between oneline blocks. Always group onelines first, then place
multiline block entries. This rule applies to all kind of definitions,
including examples it
, before
blocks or let
definitions as well.
describe "#call" do
let(:instance) { described_class.new(order, attributes) }
let(:price) { "137.00" } # ^^^ No spaces between one line blocks
let(:order) { instance_double(Order) }
let(:attributes) do # ^^^ Space here
{
uuid: uuid,
order_uuid: "a1dfdf7c-39d3-49b6-9ba1-1f0fc4f900e8",
amount: "%.2<price>f".format(price: price),
headers: {
"HTTP_VERSION" => "1",
},
}
end
let(:params) do # ^^^ And here
# ...
end
context "with missing argument" do # ^^^ And obviously here
# ...
end
end
Do not pass complex object as a parameters to methods which schedule the jobs. Sidekiq params are serialized and stored in Redis. When the job has to be done in context of some Record, always pass just the id (number or uuid), and then load the Object inside worker code.
Do not put any logic inside the typical Sidekiq worker code. Always
encapsulate expected logic into some separate Method Object. Call that
inside #perform
method. Most prefered way is actually to load expected
object then pass that object to the Method Object.
Write a simple spec for this behavior. Check if the expected object is loaded using preferred method and if the Method Object is called using expected method with provided parameteres. Test the core logic in the Method Object spec outside the worker code.
class Agent::Blocker::Worker
include Sidekiq::Worker
include RedisMutex::Macro
auto_mutex :perform, on: [:agent_id]
def perform(agent_id)
agent = Agent.find(agent_id)
Agent::Blocker.call(agent)
end
end
Encapsulate application logic inside Method Objects. Do not relay on class methods to provide functionality. Work on class instances instead. In order to simply API and testing always provide a shortcut/proxy class method.
It sole purpose should be to instantiate the class with given arguments and then call the main instance method. In general case the main method should be called #call.
Always make internal methods private. Ideal Method Object implementation should have only one public method.
Even if the logic is extremely simple and it takes only one line, do not use class methods to provide implementation. Create snippets and marcos in your development evironment to make the process of createing new classes more easy and frictionless.
# Divide two numbers or return zero if the second argument is
# equal to zero.
class Divider
def self.call(*args)
new(*args).call
end
def initialize(arg1, arg2)
self.arg1 = arg1
self.arg2 = arg2
end
def call
return 0 if arg2.zero?
arg1 / arg2
end
private
attr_accessor :arg1, :arg2
end
If instead of return value you need to work on class instance just modify the proxy class method to execute the logic and then return an instance instead. This approach shouldn't be used often outside of the facade / action objects used in controllers.
def self.call(*args)
instance = new(*args)
instance.call
instance
end
Always create spec files for every method object. If for some reason you can't provide spec for given method object at the time create pending spec file with the corresponding path.
RSpec.describe Divider do
describe ".call" do
subject { described_class }
it { should forward_to_instance(:call).with_2_args }
end
describe "#call" do
let(:service) { described_class.new(10, arg) }
context "with non-zero argument" do
let(:arg) { 5 }
it "returns division result" do
expect(service.call).to eq 2
end
end
context "with zero argument" do
let(:arg) { 0 }
it "returns 0" do
expect(service.call).to eq 0
end
end
end
end
If the logic provides vastly different paths based on the input always create corresponding branches using contexts (zero / non zero input in the Divider example). Use common sense for amount of context. In general use border values and some middle values when dealing with range arguments.
Always provide isloated envorinment for method object specs. If there are calls to external objects inside a method object - stub the calls with resonable values. Always check if the calls was actually made in expected conditions and check if the expected value was passed to the call.
Preffer stubs instead of dependency injection. Due to nature of Ruby semantics heavy dependency injection makes the code unreadable hard to reason about. Verified stubs also provide basic consistency checks without resorting to integration testing.
# Iterate over provided collection of orders and return
# only the orders which are finished.
class Order::Finished::Selector
def self.call(*args)
new(*args).call
end
def initialize(orders)
self.orders = orders
end
def call
orders.select do |order|
Order::Finished::Verifier.call(order)
end
end
private
attr_accessor :orders
end
Even though orders in this method object are collection of objects the logic doesn't interact with the objects at all. They are only passed to external class and selected based on the result. When creating specs for such cases there is no need to create those objects or even use doubles. In many cases pure symbols representing the abstract list element will do just fine. It will make the specs simpler and more clear. You could just as well use doubles instead, but if you are just passing the entity around prefer symbols over anything else.
require "rails_helper"
RSpec.describe Order::Finished::Selector do
describe ".call" do
subject { described_class }
it { should forward_to_instance(:call).with_1_arg }
end
describe "#call" do
let(:orders) { %i[order1 order2 order3] }
before do
allow(Order::Finished::Verifier)
.to receive(:call).and_return(true, false, true)
end
it "returns only finished orders" do
expect(service.call).to eq [:order1, :order3]
end
it "calls the verifier" do
service.call
expect(Order::Finished::Verifier)
.to have_received(:call)
.with(:order1)
.with(:order2)
.with(:order3)
end
end
end
Do not add custom (non-rest) controller actions. Say you have a
PaymentsController
and want to add a custom action for a web hook:
class Admin::PaymentsController < Admin::BaseController
before_action :authenticate_user!
def new
# ...
end
def create
# ...
end
def update_payment
# web hook logic
end
end
Instead of adding a custom method to an existing controller, create a new
controller with a REST action, that would correspond to update_payment
:
class WebHooksController < ApplicationController
def update
# web hook logic
end
end
This would allow to use resourceful routes instead of defining a custom route:
resources :web_hooks, only: [:update]
Details:
Whenever record is being created or updated use .create!
or #update!
to
prevent silent failing. Use #update
or .create
only with corresponding if
check.
Do not use string concatenation and interpolation, use Kernel.format
(Kernel%sprintf
's alias) instead. Benefit of this approach might
not be obvious for short strings, but we want't to keep the code base
consistent. String interpolations can quickly lead to very dense code with
logic embedded inside the #{...}
calls.
format("Hello %<name>s!", name: "Tom") # => "Hello Tom!"
Use heredocs
for strings longer than maximum line length.
def command
format(
<<~TXT.squish,
ssh -o "StrictHostKeyChecking=no" -l account%<seat>s -T
192.168.186.%<seat>s 'xwd -root -display :0|convert xwd:- png:-'
TXT
seat: user.seat,
)
end
Details:
- https://ruby-doc.org/core-2.6.1/Kernel.html#method-i-format
- https://batsov.com/articles/2013/06/27/the-elements-of-style-in-ruby-number-2-favor-sprintf-format-over-string-number-percent/
- https://www.rubyguides.com/2012/01/ruby-string-formatting/
One of the biggest pains when using structure.sql
is ensuring that only the
required changes get committed to that file. When you pull someone’s branch
and run the migrations specific to that branch, your structure.sql
will now
contain some changes. Say, you then go back to working on your own branch and
generate a new migration. Your structure.sql
file will now contain both your
branch’s and the other branch’s changes. This only gets worse with growing
number of migrations from different not-yet-or-never-to-be merged branches.
Here is the strategy to ensure that structure.sql
file only contains the
necessary changes to a specific branch. Once you are done working on a branch
that contains migrations, make sure you run rails db:rollback STEP=n
, where
n
is the number of migrations in that branch. This will ensure your database
structure reverts to its original state.
Details:
Projects related to style guide and code formatting:
- https://github.com/testdouble/standard
- https://github.com/samphippen/rubyfmt
- https://github.com/rubocop-hq/rubocop
- https://github.com/uohzxela/clean-code-ruby
- https://github.com/airbnb/ruby
- https://github.com/rubocop-hq/ruby-style-guide
We try to align our coding style with general Rubocop defaults. This repository contains generic Rubocop configuration used in out projects. Make sure you have an operational linter in your code editor, so you commit as little mismatched code as possible. We have a HoundCi watching our repos, but comments generate noise, so make sure to double check your style before pushing your commits outside.
We use soft-tabs with a two space indent. Never ident your code for more than one level at each step.
# This is GOOD
kind =
case year
when 1850..1889 then "Blues"
when 1890..1909 then "Ragtime"
else "Jazz"
end
# This is BAD
kind = case year
when 1850..1889 then "Blues"
when 1890..1909 then "Ragtime"
else "Jazz"
end
# This is GOOD
def self.create_translation(
phrase_id,
phrase_key,
value,
user_id
)
...
end
# This is BAD
def self.create_translation(phrase_id, phrase_key
value, user_id)
...
end
# This is BAD
def self.create_translation(phrase_id,
phrase_key,
value,
user_id)
...
end
Always limit lines to 80 characters in Ruby code. There are no exceptions to this rule. Any auto-generated code (Rails/RSpec/gems/etc) are excluded from checks, so we do not correct any style offences there. If you have to include any static data with long lines in your code move them to separate files (YAML/JSON).
Views are allowed to have more than 80 long lines, but try to limit long lines there if possible, especially in non-erb templates (Haml/Slim).
This is an excerpt from Rubocop style guide.
Why Bother with 80 characters in a World of Modern Widescreen Displays?
A lot of people these days feel that a maximum line length of 80 characters is just a remnant of the past and makes little sense today. After all - modern displays can easily fit 200+ characters on a single line. Still, there are some important benefits to be gained from sticking to shorter lines of code.
First, and foremost - numerous studies have shown that humans read much faster vertically and very long lines of text impede the reading process. As noted earlier, one of the guiding principles of this style guide is to optimize the code we write for human consumption.
Additionally, limiting the required editor window width makes it possible to have several files open side-by-side, and works well when using code review tools that present the two versions in adjacent columns.
The default wrapping in most tools disrupts the visual structure of the code, making it more difficult to understand. The limits are chosen to avoid wrapping in editors with the window width set to 80, even if the tool places a marker glyph in the final column when wrapping lines. Some web based tools may not offer dynamic line wrapping at all.
Some teams strongly prefer a longer line length. For code maintained exclusively or primarily by a team that can reach agreement on this issue, it is okay to increase the line length limit up to 100 characters, or all the way up to 120 characters. Please, restrain the urge to go beyond 120 characters.
If possible limit your line length even more, to 60 characters. Longer lines tend to pack a lot of logic into very small visual space. We try to break our code into very minimal pieces and doing long method chains in one line defeates this purpose.
Do not cripple your code in order to meet line length requirement. If you have a hard time fitting into a line (or a method), this is a signal you should expand your code (or extract some logic into other class/method).
# This is GOOD
users.map do |user|
format(
"Hello %<firstname>s %<lastname>s!",
firstname: user.firstname,
lastname: user.lastname,
)
end
# This is BAD
users.map { format("Hello %<fn>s %<ln>s!", fn: _1.firstname, ln: _1.lastname) }
This section contains links to usefull articles, books, videos, podcasts and other resources.
- Why We Killed Our End-to-End Test Suite (2021)
- Responsible Monkeypatching in Ruby (2021)
- Hi! It's me, your flaky system test again (2021)
- Reasons why SELECT * is bad for SQL performance (2020)
- ROM + Dry Showcase: Part 1 (2020)
- The worst mistake of computer science - Paul Draper (2019)
- 7 Patterns to Refactor Fat ActiveRecord Models - Bryan Helmkamp (2012)
- The 80/24 rule (2019)
- Types of Coupling (2015)
- Let's Not (2019)
- Get Rid of That Code Smell – Control Couple (2012)
- Good design and type safety in Yahtzee | Haskell (2019
- The Guest: A Guide To Code Hospitality by Nadia Odunayo (2016)
- AnemicDomainModel (2003)
- Beware of “service objects” in Rails (2019)
- When To Use Microservices - Sam Newman & Martin Fowler (2020)
- Mastering Chaos - A Netflix Guide to Microservices - Josh Evants (2016)
- Nothing is Something - Sandi Metz (2015)
- Refactoring Ruby with Monads (2014)
- Boundaries - Gary Bernhardt (2012)
- Y Not - Jim Weirch (2012)
- Practical Object-Oriented Design (POODR) - Sandi Metz
- Rails 5 Test Prescriptions - Noel Rappin
- Everyday Rails Testing with RSpec - Aaron Sumner
- Exceptional Ruby - Avdi Grimm
Podcasts related to Ruby or Ruby on Rails in one way or another. Mostly due to hosts / guests beeing Rails developers:
General development (or tech business) related podcasts:
- The Changelog
- Full Stack Radio
- Software Engineering Daily
- Giant Robots Smashing Into Other Giant Robots
- Syntax
- Greater than Code
- Soft Skills Engineering
- Developer Tea
- Founder Quest
Podcasts focused on non-ruby/rails related technologies:
Discontinued, but worth mentioning: