rails

Rails

• Prefer Time.current over Time.now, Date.current over Date.today ^link

• Extract long chains of unnamed conditions into named scopes or class methods ^link

  Example

  ## Bad
  def index
    @recipes = Recipe.where(published: true).where.not(approved_at: nil)
  end
  
  ## Good
  class Recipe
    scope :published, -> { where(published: true) }
    scope :approved, -> { where.not(approved_at: nil) }
    scope :live, -> { published.approved }
  end
  
  def index
    @recipes = Recipe.live
  end

• Prefer class method over scope if the scope takes an argument or spans multiple lines ^link

  Example

  ## Bad
  class Recipe
    scope :published_on, -> (date) { where(published_on: date) }
  end
  
  ## Good
  class Recipe
    def self.published_on(date)
      where(published_on: date)
    end
  end
  
  ## Bad
  class Recipe
    scope :fresh, -> {
      recently_active.
        published(4.weeks.ago).
        popular.
        approved
    }
  end
  
  ## Good
  class Recipe
    def self.fresh
      recently_active.
        published(4.weeks.ago).
        popular.
        approved
    end
  end

• Use a query object if a scope can be clarified by extracting private methods/needs additional state ^link

  Example

  ## Bad
  class Recipe
    def self.local
      radius_maximum = Config.radius_maximum
      distance = radius_maximum  / (Math::PI * 6371)
      where(distance: distance)
    end
  end
  
  ## Good
  class Recipe
    def self.local
      GeoSquareQuery.new(self).to_relation
    end
  end
  
  class GeoSquareQuery
    EARTH_RADIUS_IN_KM = 6371
  
    def initialize(relation)
      @relation = relation
    end
  
    def to_relation
      relation.where(distance: distance)
    end
  
    private
  
      attr_reader :relation
  
      def distance
        radius_maximum / (Math::PI * EARTH_RADIUS_IN_KM)
      end
  
      def radius_maximum
        Config.radius_maximum
      end
  end

• Prefer private methods over before_action to set instance variables ^{link explanation}

  Example (when setting context)

  ## Bad
  class EntriesController < ApplicationController
    before_action :set_contest
  
    def index
      @entries = @contest.entries
    end
  
    private
  
      def set_contest
        @contest = Contest.find(params[:contest_id])
      end
  end
  
  ## Good
  class EntriesController < ApplicationController
    def index
      @entries = contest.entries
    end
  
    private
  
      def contest
        @_contest ||= Contest.find(params[:contest_id])
      end
  end

  Example (when assiging vars to use in view)

  ## Bad
  class EntriesController < ApplicationController
    before_action :set_entry
  
    def show
    end
  
    private
  
      def set_entry
        @entry = contest.entries.find(params[:id])
      end
  end
  
  ## Good
  class EntriesController < ApplicationController
    def show
      @entry = contest.entries.find(params[:id])
    end
  end

  Example (when particular complicated/used more than once in the controller)

  ## If particular complicated/used more than once in the controller:
  class EntriesController < ApplicationController
    def show
      @entry = entry
    end
  
    def update
      @entry = entry
      if @entry.update(entry_params)
        redirect_to @entry
      else
        render :edit
      end
    end
  
    private
  
      def entry
        @_entry ||=  contest.entries.published.active.find(params[:id])
      end
  end

• Prefer adding new controllers with RESTful actions over using custom actions ^{link explanation}

  Example

  ## Bad
  class UserController < ApplicationController
    def ban
      user.ban
    end
  
    def unban
      user.unban
    end
  end
  
  ## Good
  class BansController < ApplicationController
    def create
      user.ban
    end
  
    def destroy
      user.unban
    end
  end

• Use fully qualified i18n name. eg. recipes.show.title rather than shorthand .title ^link

• Use config/locales/experiments.XX.yml files to store experiment strings that do not need to be translated in all languages yet. ^link

  Explanation

  Strings that have been added to config/locales/en.yml get sent to OneSky for translation, which notifies the translation team, who will then attempt to add translations, even in the case of an experiment that does not require the effort. Keeping those strings in a separate translation file (as opposed to using the default: option) provides the following benefits:

  1. There is no need to maintain the same default strings in multiple locations in the code.
  2. If the experiment graduates to a permanent feature, graduating the translation strings requires a lower effort.

• Scope very generic i18n phrases under common. eg. 'Delete' or 'Cookpad' are good candidates for common ^link

• Only use i18n pluralization to preserve correct grammar ^link

  Example

  # Pluralization rules vary from language to language and keys are automatically added and
  # removed from the translation files. 
  # The only key present in all languages is `other`
  # Ref: https://unicode-org.github.io/cldr-staging/charts/latest/supplemental/language_plural_rules.html
  
  # Don't use i18n `zero:` key to display a "no results" message.
  # Bad
  search_results:
    zero: "There were no results"
    one: "1 recipe found"
    other: "%{count} recipes found"
  
  # Use a separate key for the "no results" message instead.
  # Good
  search_results:
    one: "1 recipe found"
    other: "%{count} recipes found"
  search_no_results: "There were no results"
  
  # Don't use i18n pluralization for controlling application logic.
  # Bad
  reactions:
    one: "%{name} reacted"
    other: "%{name} and others reacted"
  
  # Good 
  reactions:
    one: "%{count} reaction"
    other: "%{count} reactions"

• Don't include HTML in locale file ^{link explanation}

  Example

  Bad

  current_time_html: "<strong>Current time:</strong> %{time}"

  <%= t("current_time_html", time: Time.current) %>

  Good

  current_time:
    label: "Current time:"
    label_time_html: "%{label} %{time}"

  <%= t("current_time.label_time_html", label: content_tag(:strong, t("current_time.label")), time: Time.current) %>

• Service objects should have a single public method #run, which does not accept arguments ^link

  Example

  ## Bad
  class ChatMessage
    def run(chat:, :body)
      chat.messages.create(body: body)
    end
  end
  
  ## Good
  class ChatMessage
    def initialize(chat:, body:)
      @chat = chat
      @body = body
    end
  
    def run
      chat.messages.create(body: body)
    end
  
    private
      attr_reader :chat, :body
  end

• If a service object needs to return a value, prefer a meaningful object, e.g. an invalid model over false ^link

  Example

  ## Bad
  def run
    message = chat.messages.new(body: body)
    message.save # true/false
  end
  
  ## Good
  def run
    message = chat.messages.new(body: body)
    message.save
    message
  end

• Prefer local variables for a partial over instance variables ^link

  Example

  <!-- Bad -->
  
  <!-- app/views/users/show.html.erb -->
  <%= render "users/follows_count" %>
  
  <!-- app/views/users/_follows_count.html.erb -->
  <div>
    <strong><%= @user.name %></strong><br>
    <span><%= @user.followers.size %> Followers</span>
    <span><%= @user.followees.size %> Following</span>
  </div>
  
  <!-- Good -->
  
  <!-- app/views/users/show.html.erb -->
  <%= render "users/follows_count", user: @user %>
  
  <!-- app/views/users/_follows_count.html.erb -->
  <div>
    <strong><%= user.name %></strong><br>
    <span><%= user.followers.size %> Followers</span>
    <span><%= user.followees.size %> Following</span>
  </div>

• Follow consistent ERB indent style ^link

  Example

  <!-- Bad: ERB tag closer on its own line -->
  <%= render "accounts/header",
        left_navigation: link_to(...),
        dismissable: true
  %>
  
  <!-- Bad: ERB tag opener & closer on their own line -->
  <%=
    render "accounts/header",
      left_navigation: link_to(...),
      dismissable: true
  %>
  
  <!-- Bad: Wrong indentation for lines after first inside ERB tag -->
  <%= render "accounts/header",
      left_navigation: link_to(...),
      dismissable: true %>
  
  <!-- Good -->
  <%= render "accounts/header",
    left_navigation: link_to(...),
    dismissable: true %>

• Leverage top-down development and feature toggles to keep pull requests small. ^{link explanation}