Rails 风格指南

# config/environments/production.rb
# Precompile additional assets (application.js, application.css,
#and all non-JS/CSS are already added)
config.assets.precompile += %w( rails_admin/rails_admin.css rails_admin/rails_admin.js )

# good
config.load_defaults 6.1

Rails::Application.config_for(:yaml_file)

# bad
get 'subscriptions/:id/unsubscribe'
resources :subscriptions

# good
resources :subscriptions do
  get 'unsubscribe', on: :member
end

# bad
get 'photos/search'
resources :photos

# good
resources :photos do
  get 'search', on: :collection
end

resources :subscriptions do
  member do
    get 'unsubscribe'
    # more routes
  end
end

resources :photos do
  collection do
    get 'search'
    # more routes
  end
end

class Post < ApplicationRecord
  has_many :comments
end

class Comment < ApplicationRecord
  belongs_to :post
end

# routes.rb
resources :posts do
  resources :comments
end

resources :posts, shallow: true do
  resources :comments do
    resources :versions
  end
end

namespace :admin do
  # Directs /admin/products/* to Admin::ProductsController
  # (app/controllers/admin/products_controller.rb)
  resources :products
end

# very bad
match ':controller(/:action(/:id(.:format)))'

# bad
class UsersController < ApplicationController
  before_action :require_login, only: :export
end

# good
class UsersController < ApplicationController
  before_action :require_login, only: :export

  def export
  end
end

# very bad
class ProductsController < ApplicationController
  def index
    render inline: "<% products.each do |p| %><p><%= p.name %></p><% end %>", type: :erb
  end
end

# good
## app/views/products/index.html.erb
<%= render partial: 'product', collection: products %>

## app/views/products/_product.html.erb
<p><%= product.name %></p>
<p><%= product.price %></p>

## app/controllers/products_controller.rb
class ProductsController < ApplicationController
  def index
    render :index
  end
end

# bad - sets MIME type to `text/html`
...
render text: 'Ruby!'
...

# bad - requires explicit MIME type declaration
...
render text: 'Ruby!', content_type: 'text/plain'
...

# good - short and precise
...
render plain: 'Ruby!'
...

# bad
...
render status: 403
...

# good
...
render status: :forbidden
...

class Message
  include ActiveModel::Model

  attr_accessor :name, :email, :content, :priority

  validates :name, presence: true
  validates :email, format: { with: /\A[-a-z0-9_+\.]+\@([-a-z0-9]+\.)+[a-z0-9]{2,4}\z/i }
  validates :content, length: { maximum: 500 }
end

class Message
  include ActiveModel::Model
  include ActiveModel::Attributes

  attribute :name, :string
  attribute :email, :string
  attribute :content, :string
  attribute :priority, :integer

  validates :name, presence: true
  validates :email, format: { with: /\A[-a-z0-9_+\.]+\@([-a-z0-9]+\.)+[a-z0-9]{2,4}\z/i }
  validates :content, length: { maximum: 500 }
end

# bad - don't do this if you can modify the schema
class Transaction < ApplicationRecord
  self.table_name = 'order'
  ...
end

class Transaction < ApplicationRecord
  # bad - it may overwrite previous assignments
  self.ignored_columns = %i[legacy]

  # good - the value is appended to the list
  self.ignored_columns += %i[legacy]
  ...
end

class Transaction < ApplicationRecord
  # bad - implicit values - ordering matters
  enum type: %i[credit debit]

  # good - explicit values - ordering does not matter
  enum type: {
    credit: 0,
    debit: 1
  }
end

class User < ApplicationRecord
  # keep the default scope first (if any)
  default_scope { where(active: true) }

  # constants come up next
  COLORS = %w(red green blue)

  # afterwards we put attr related macros
  attr_accessor :formatted_date_of_birth

  attr_accessible :login, :first_name, :last_name, :email, :password

  # Rails 4+ enums after attr macros
  enum role: { user: 0, moderator: 1, admin: 2 }

  # followed by association macros
  belongs_to :country

  has_many :authentications, dependent: :destroy

  # and validation macros
  validates :email, presence: true
  validates :username, presence: true
  validates :username, uniqueness: { case_sensitive: false }
  validates :username, format: { with: /\A[A-Za-z][A-Za-z0-9._-]{2,19}\z/ }
  validates :password, format: { with: /\A\S{8,128}\z/, allow_nil: true }

  # next we have callbacks
  before_save :cook
  before_save :update_username_lower

  # other macros (like devise's) should be placed after the callbacks

  ...
end

# not so good - using has_and_belongs_to_many
class User < ApplicationRecord
  has_and_belongs_to_many :groups
end

class Group < ApplicationRecord
  has_and_belongs_to_many :users
end

# preferred way - using has_many :through
class User < ApplicationRecord
  has_many :memberships
  has_many :groups, through: :memberships
end

class Membership < ApplicationRecord
  belongs_to :user
  belongs_to :group
end

class Group < ApplicationRecord
  has_many :memberships
  has_many :users, through: :memberships
end

# bad
def amount
  read_attribute(:amount) * 100
end

# good
def amount
  self[:amount] * 100
end

# bad
def amount
  write_attribute(:amount, 100)
end

# good
def amount
  self[:amount] = 100
end

# bad
validates_presence_of :email
validates_length_of :email, maximum: 100

# good
validates :email, presence: true, length: { maximum: 100 }

# good
validate :expiration_date_cannot_be_in_the_past
validate :discount_cannot_be_greater_than_total_value
validate :ensure_same_topic_is_chosen

# also good - explicit prefix
validate :validate_birthday_in_past
validate :validate_sufficient_quantity
validate :must_have_owner_with_no_other_items
validate :must_have_shipping_units

# bad
validate :birthday_in_past
validate :owner_has_no_other_items

# bad
validates :email, :password, presence: true
validates :email, length: { maximum: 100 }

# good
validates :email, presence: true, length: { maximum: 100 }
validates :password, presence: true

# bad
class Person
  validates :email, format: { with: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i }
end

# good
class EmailValidator < ActiveModel::EachValidator
  def validate_each(record, attribute, value)
    record.errors[attribute] << (options[:message] || 'is not a valid email') unless value =~ /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i
  end
end

class Person
  validates :email, email: true
end

class User < ApplicationRecord
  scope :active, -> { where(active: true) }
  scope :inactive, -> { where(active: false) }

  scope :with_orders, -> { joins(:orders).select('distinct(users.id)') }
end

class User < ApplicationRecord
  def self.with_orders
    joins(:orders).select('distinct(users.id)')
  end
end

# bad
class Person
  after_commit :after_commit_callback
  before_validation :before_validation_callback
end

# good
class Person
  before_validation :before_validation_callback
  after_commit :after_commit_callback
end

# bad
Article.first.decrement!(:view_count)
DiscussionBoard.decrement_counter(:post_count, 5)
Article.first.increment!(:view_count)
DiscussionBoard.increment_counter(:post_count, 5)
person.toggle :active
product.touch
Billing.update_all("category = 'authorized', author = 'David'")
user.update_attribute(:website, 'example.com')
user.update_columns(last_request_at: Time.current)
Post.update_counters 5, comment_count: -1, action_count: 1

# good
user.update_attributes(website: 'example.com')

# bad
Person.all.each do |person|
  person.do_awesome_stuff
end

Person.where('age > 21').each do |person|
  person.party_all_night!
end

# good
Person.find_each do |person|
  person.do_awesome_stuff
end

Person.where('age > 21').find_each do |person|
  person.party_all_night!
end

# bad (roles will be deleted automatically even if super_admin? is true)
has_many :roles, dependent: :destroy

before_destroy :ensure_deletable

def ensure_deletable
  raise "Cannot delete super admin." if super_admin?
end

# good
has_many :roles, dependent: :destroy

before_destroy :ensure_deletable, prepend: true

def ensure_deletable
  raise "Cannot delete super admin." if super_admin?
end

# bad
class Post < ApplicationRecord
  has_many :comments
end

# good
class Post < ApplicationRecord
  has_many :comments, dependent: :destroy
end

# bad
user.create(name: 'Bruce')

# bad
user.save

# good
user.create!(name: 'Bruce')
# or
bruce = user.create(name: 'Bruce')
if bruce.persisted?
  ...
else
  ...
end

# good
user.save!
# or
if user.save
  ...
else
  ...
end

# bad - param will be interpolated unescaped
Client.where("orders_count = #{params[:orders]}")

# good - param will be properly escaped
Client.where('orders_count = ?', params[:orders])

# okish
Client.where(
  'orders_count >= ? AND country_code = ?',
  params[:min_orders_count], params[:country_code]
)

# good
Client.where(
  'orders_count >= :min_orders_count AND country_code = :country_code',
  min_orders_count: params[:min_orders_count], country_code: params[:country_code]
)

# bad
User.where(id: id).take!

# bad
User.find_by_id!(id)

# bad
User.find_by!(id: id)

# good
User.find(id)

# bad
User.where(email: email).take
User.where(first_name: 'Bruce', last_name: 'Wayne').take

# bad
User.find_by_email(email)
User.find_by_first_name_and_last_name('Bruce', 'Wayne')

# good
User.find_by(email: email)
User.find_by(first_name: 'Bruce', last_name: 'Wayne')

# bad
User.where("name = ?", name)

# good
User.where(name: name)

# bad
User.where("id != ?", id)

# good
User.where.not(id: id)

# bad
Post.left_joins(:author).where(authors: { id: nil })

# good
Post.where.missing(:author)

# bad
scope :chronological, -> { order(id: :asc) }

# good
scope :chronological, -> { order(created_at: :asc) }

# bad
User.all.map(&:name)

# bad
User.all.map { |user| user[:name] }

# good
User.pluck(:name)

# bad
User.pluck(:name).first

# bad
User.first.name

# good
User.pick(:name)

# bad
User.pluck(:id)

# good
User.ids

User.find_by_sql(<<-SQL.squish)
  SELECT
    users.id, accounts.plan
  FROM
    users
  INNER JOIN
    accounts
  ON
    accounts.user_id = users.id
  # further complexities...
SQL

SELECT\n    users.id, accounts.plan\n  FROM\n    users\n  INNER JOIN\n    accounts\n  ON\n    accounts.user_id = users.id

# bad
User.count

# good
User.all.size

# good - if you really need to load all users into memory
User.all.length

# bad
User.where("created_at >= ?", 30.days.ago).where("created_at <= ?", 7.days.ago)
User.where("created_at >= ? AND created_at <= ?", 30.days.ago, 7.days.ago)
User.where("created_at >= :start AND created_at <= :end", start: 30.days.ago, end: 7.days.ago)

# good
User.where(created_at: 30.days.ago..7.days.ago)

# bad
User.where("created_at >= ?", 7.days.ago)

# good
User.where(created_at: 7.days.ago..)

# note - ranges are inclusive or exclusive of their ending, not beginning
User.where(created_at: 7.days.ago..)  # produces >=
User.where(created_at: 7.days.ago...) # also produces >=
User.where(created_at: ..7.days.ago)  # inclusive: produces <=
User.where(created_at: ...7.days.ago) # exclusive: produces <

# okish - there is no range syntax that would denote exclusion at the beginning of the range
Customer.where("purchases_count > :min AND purchases_count <= :max", min: 0, max: 5)

# bad
User.where.not(status: 'active', plan: 'basic')

# good
User.where.not('status = ? AND plan = ?', 'active', 'basic')

# bad
User.all.find(id)
User.all.order(:created_at)
users.all.where(id: ids)
user.articles.all.order(:created_at)

# good
User.find(id)
User.order(:created_at)
users.where(id: ids)
user.articles.order(:created_at)

# bad - application enforced default value
class Product < ApplicationRecord
  def amount
    self[:amount] || 0
  end
end

# good - database enforced
class AddDefaultAmountToProducts < ActiveRecord::Migration
  def change
    change_column_default :products, :amount, 0
  end
end

# bad - boolean without a default value
add_column :users, :active, :boolean

# good - boolean with a default value (`false` or `true`) and with restricted `NULL`
add_column :users, :active, :boolean, default: true, null: false
add_column :users, :admin, :boolean, default: false, null: false

# bad - does not add foreign keys
create_table :comment do |t|
  t.references :article
  t.belongs_to :user
  t.integer :category_id
end

# good
create_table :comment do |t|
  t.references :article, foreign_key: true
  t.belongs_to :user, foreign_key: true
  t.references :category, foreign_key: { to_table: :comment_categories }
end

# the old way
class AddNameToPeople < ActiveRecord::Migration
  def up
    add_column :people, :name, :string
  end

  def down
    remove_column :people, :name
  end
end

# the new preferred way
class AddNameToPeople < ActiveRecord::Migration
  def change
    add_column :people, :name, :string
  end
end

# db/migrate/<migration_file_name>.rb
# frozen_string_literal: true

# bad
class ModifyDefaultStatusForProducts < ActiveRecord::Migration
  def change
    old_status = 'pending_manual_approval'
    new_status = 'pending_approval'

    reversible do |dir|
      dir.up do
        Product.where(status: old_status).update_all(status: new_status)
        change_column :products, :status, :string, default: new_status
      end

      dir.down do
        Product.where(status: new_status).update_all(status: old_status)
        change_column :products, :status, :string, default: old_status
      end
    end
  end
end

# good
# Define `table_name` in a custom named class to make sure that you run on the
# same table you had during the creation of the migration.
# In future if you override the `Product` class and change the `table_name`,
# it won't break the migration or cause serious data corruption.
class MigrationProduct < ActiveRecord::Base
  self.table_name = :products
end

class ModifyDefaultStatusForProducts < ActiveRecord::Migration
  def change
    old_status = 'pending_manual_approval'
    new_status = 'pending_approval'

    reversible do |dir|
      dir.up do
        MigrationProduct.where(status: old_status).update_all(status: new_status)
        change_column :products, :status, :string, default: new_status
      end

      dir.down do
        MigrationProduct.where(status: new_status).update_all(status: old_status)
        change_column :products, :status, :string, default: old_status
      end
    end
  end
end

# bad
class AddFkArticlesToAuthors < ActiveRecord::Migration
  def change
    add_foreign_key :articles, :authors
  end
end

# good
class AddFkArticlesToAuthors < ActiveRecord::Migration
  def change
    add_foreign_key :articles, :authors, name: :articles_author_id_fk
  end
end

# bad
class DropUsers < ActiveRecord::Migration
  def change
    drop_table :users
  end
end

# good
class DropUsers < ActiveRecord::Migration
  def up
    drop_table :users
  end

  def down
    create_table :users do |t|
      t.string :name
    end
  end
end

# good
# In this case, block will be used by create_table in rollback
# https://api.rubyonrails.net.cn/classes/ActiveRecord/ConnectionAdapters.html#method-i-drop_table
class DropUsers < ActiveRecord::Migration
  def change
    drop_table :users do |t|
      t.string :name
    end
  end
end

<!-- bad -->
<!-- app/views/courses/show.html.erb -->
<%= render 'course_description' %>
<!-- app/views/courses/_course_description.html.erb -->
<%= @course.description %>

<!-- good -->
<!-- app/views/courses/show.html.erb -->
<%= render 'course_description', course: @course %>
<!-- app/views/courses/_course_description.html.erb -->
<%= course.description %>

en:
  activerecord:
    models:
      user: Member
    attributes:
      user:
        name: 'Full name'

# config/application.rb
config.i18n.load_path += Dir[Rails.root.join('config', 'locales', '**', '*.{rb,yml}')]

en:
  users:
    show:
      title: 'User details page'

# bad
= t 'users.show.title'

# good
= t '.title'

# bad
I18n.t :record_invalid, scope: [:activerecord, :errors, :messages]

# good
I18n.t :record_invalid, scope: 'activerecord.errors.messages'
I18n.t 'activerecord.errors.messages.record_invalid'

# bad
I18n.t :title, scope: :invitation

# good
I18n.t 'title.invitation'

# config/environments/development.rb

config.action_mailer.raise_delivery_errors = true

# config/environments/development.rb

config.action_mailer.smtp_settings = {
  address: 'localhost',
  port: 1025,
  # more settings
}

# config/environments/development.rb
config.action_mailer.default_url_options = { host: "#{local_ip}:3000" }

# config/environments/production.rb
config.action_mailer.default_url_options = { host: 'your_site.com' }

# in your mailer class
default_url_options[:host] = 'your_site.com'

# in your mailer class
default from: 'Your Name <info@your_site.com>'

# in your mailer class
default from: email_address_with_name('info@your_site.com', 'Your Name')

# config/environments/test.rb

config.action_mailer.delivery_method = :test

# config/environments/development.rb, config/environments/production.rb

config.action_mailer.delivery_method = :smtp

# bad
obj.try! :fly

# good
obj&.fly

# bad
'the day'.starts_with? 'th'
'the day'.ends_with? 'ay'

# good
'the day'.start_with? 'th'
'the day'.end_with? 'ay'

# bad
(1..50).to_a.forty_two
1.in? [1, 2]
'day'.in? 'the day'

# good
(1..50).to_a[41]
[1, 2].include? 1
'the day'.include? 'day'

# bad - String#inquiry
ruby = 'two'.inquiry
ruby.two?

# good
ruby = 'two'
ruby == 'two'

# bad - Array#inquiry
pets = %w(cat dog).inquiry
pets.gopher?

# good
pets = %w(cat dog)
pets.include? 'cat'

# bad
!array.include?(2)
!hash.include?(:key)
!string.include?('substring')

# good
array.exclude?(2)
hash.exclude?(:key)
string.exclude?('substring')

# bad
<<EOS.strip_heredoc
  some text
EOS

# bad
<<-EOS.strip_heredoc
  some text
EOS

# good
<<~EOS
  some text
EOS

# bad
time.to_formatted_s(:db)
date.to_formatted_s(:db)
datetime.to_formatted_s(:db)
42.to_formatted_s(:human)

# good
time.to_fs(:db)
date.to_fs(:db)
datetime.to_fs(:db)
42.to_fs(:human)

config.time_zone = 'Eastern European Time'
# optional - note it can be only :utc or :local (default is :utc)
config.active_record.default_timezone = :local

# bad
Time.parse('2015-03-02 19:05:37') # => Will assume time string given is in the system's time zone.

# good
Time.zone.parse('2015-03-02 19:05:37') # => Mon, 02 Mar 2015 19:05:37 EET +02:00

# bad - assumes time string given is in the system's time zone.
'2015-03-02 19:05:37'.to_time

# good
Time.zone.parse('2015-03-02 19:05:37') # => Mon, 02 Mar 2015 19:05:37 EET +02:00

# bad
Time.now # => Returns system time and ignores your configured time zone.

# good
Time.zone.now # => Fri, 12 Mar 2014 22:04:47 EET +02:00
Time.current # Same thing but shorter.

# bad
date.beginning_of_day..date.end_of_day
date.beginning_of_week..date.end_of_week
date.beginning_of_month..date.end_of_month
date.beginning_of_quarter..date.end_of_quarter
date.beginning_of_year..date.end_of_year

# good
date.all_day
date.all_week
date.all_month
date.all_quarter
date.all_year

# bad - It's not clear that the qualifier refers to the current time (which is the default parameter)
5.hours.since
5.hours.after
5.hours.before
5.hours.until

# good
5.hours.from_now
5.hours.ago

# bad - It's confusing and misleading to read
2.days.from_now(yesterday)
2.days.ago(yesterday)

# good
2.days.since(yesterday)
2.days.after(yesterday)
2.days.before(yesterday)
2.days.until(yesterday)

# bad - It's confusing and misleading to read
-5.hours.from_now
-5.hours.ago

# good
5.hours.ago
5.hours.from_now

# bad
Time.current - 1.minute
Time.zone.now + 2.days

# good
1.minute.ago
2.days.from_now

# bad
class MyControllerTest < ActionController::TestCase
end

# good
class MyControllerTest < ActionDispatch::IntegrationTest
end

# bad
travel_to(Time.now)
travel_to(DateTime.now)
travel_to(Time.current)
travel_to(Time.zone.now)
travel_to(Time.now.in_time_zone)
travel_to(Time.current.to_time)

# good
freeze_time