RuboCop vs Rails Best Practices — Which one is better for beginners?

These days, it’s common for users to use tests during development cycle codes. As web service such as GitHub and BitBucket have become more popular and widely used, it also has become a common practice for users to test and review their codes before merging them.

But, how about the element that we can’t verify by tests? For example, tests cannot verify the following elements, and reviewing them will need a certain amount of human resources.

The code’s complexity
Variable name validity
Code that follow frameworks
Misleadingly evaluated code
etc

So, how can we reduce the workload?

In this article, our target will be Ruby on Rails, and we will go through how to use the review tools before performing the unit tests.

The Types of Tools

Currently, there are many types of products for Ruby, and we will look at the most commonly used ones, as listed below.

RuboCop’s Characteristics

RuboCop has an inspection module called Cop that can inspect a code’s notation variation, grammar, and method complexity. Also, you can set it on / off with a threshold value from a file called .rubocop.yml. In addition, a tool called auto-correct can make corrections based on RuboCop’s ability.

Rails Best Practices’s Characteristics

rails-bestpractices.com is a best practice collection site for implementing Rails application. It checks whether or not the code follows the best practice.

You can now run the static code analysis on Ruby on Rails MVC. Apart from ActiveRecord, a standard ORM for Ruby on Rails. It also supports mongoid and mongomapper. The template engine has erb, slim, haml, rabl, and so on.

Trying it out

What we use

Ruby 2.4.2
The target is the representative of Rails application. It’s also for the Redmine 3.4.3 that hasn’t adopted the gem/rails_best_practices.
Our objective is not to activate Redmine, so we will not check its start-up.

RuboCop

Introduction

Rewriting the Gemfile

In the group :test, add the following line:

gem ‘rubocop’, require: false

Creating database.yml

When installing bundle, add database.yml using Gemfile.

production: &production
  adapter: sqlite3
  database: db/redmine.sqlite3

development:
  <<: *production

test:
  <<: *production

Introducing Gem

$ bundle install — path=vendor/bundle — binstubs

Depending on your environment, you might need ImageMagick6 in advance.

Testing the operation

$ bin/rubocop -P

We will execute it in parallel with the P option. Because RuboCop is a relatively heavy process, this is suitable for an environment with many CPUs.

The results are as follows (abbreviated):

db/migrate/20110226120112_change_repositories_password_limit.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration
^
db/migrate/20110226120112_change_repositories_password_limit.rb:3:54: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => nil, :default => ''
                                                     ^^^^^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:3:69: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => nil, :default => ''
                                                                    ^^^^^^^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:3:81: C: Metrics/LineLength: Line is too long. [82/80]
    change_column :repositories, :password, :string, :limit => nil, :default => ''
                                                                                ^^
db/migrate/20110226120112_change_repositories_password_limit.rb:7:54: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => 60, :default => ''
                                                     ^^^^^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:7:68: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => 60, :default => ''
                                                                   ^^^^^^^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:7:81: C: Metrics/LineLength: Line is too long. [81/80]
    change_column :repositories, :password, :string, :limit => 60, :default => ''
                                                                                ^

979 files inspected, 64504 offenses detected

Although this may seem surprising to the readers, if you don’t run RuboCop with detailed settings, it will recognize a large number of violations. In this case, there were 64,504 offenses that were made (by the way, an offense can also mean a crime: a joke for the “cops” out there!)

Responding to the offenses

The reasons for the offenses above are…

1. There’s no comment to specify a string as frozen (Style type cop)

2. The hash’s syntax is in ruby 1.9 style format (Style type cop)

3. One line is too long (Metrics types Cop)

Firstly, let’s tackle these.

diff --git a/db/migrate/20110226120112_change_repositories_password_limit.rb b/db/migrate/20110226120112_change_repositories_password_limit.rb
index 1ad937c7d..0d244f031 100644
--- a/db/migrate/20110226120112_change_repositories_password_limit.rb
+++ b/db/migrate/20110226120112_change_repositories_password_limit.rb
@@ -1,9 +1,10 @@
+# frozen_string_literal: true
 class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration
   def self.up
-    change_column :repositories, :password, :string, :limit => nil, :default => ''
+    change_column :repositories, :password, :string, limit: nil, default: ''
   end

def self.down
-    change_column :repositories, :password, :string, :limit => 60, :default => ''
+    change_column :repositories, :password, :string, limit: 60, default: ''
   end
 end

And now, let’s run it again.

$ bin/rubocop db/migrate/20110226120112_change_repositories_password_limit.rb
Inspecting 1 file
C

Offenses:

db/migrate/20110226120112_change_repositories_password_limit.rb:2:1: C: Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration
^
db/migrate/20110226120112_change_repositories_password_limit.rb:2:1: C: Style/Documentation: Missing top-level class documentation comment.
class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration
^^^^^

1 file inspected, 2 offenses detected

It seems that there are two new offenses due to the changes made.

There is no blank line after the Magic Comment (Layout style cop)
There is no description for the class for TopLevel (Style type cop)

Although a few details were detected, let’s make a few more adjustments so they will not be detected.

diff --git a/db/migrate/20110226120112_change_repositories_password_limit.rb b/db/migrate/20110226120112_change_repositories_password_limit.rb
index 1ad937c7d..29c91ad56 100644
--- a/db/migrate/20110226120112_change_repositories_password_limit.rb
+++ b/db/migrate/20110226120112_change_repositories_password_limit.rb
@@ -1,9 +1,12 @@
+# frozen_string_literal: true
+
+# @class Abracadabra
 class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration
   def self.up
-    change_column :repositories, :password, :string, :limit => nil, :default => ''
+    change_column :repositories, :password, :string, limit: nil, default: ''
   end

   def self.down
-    change_column :repositories, :password, :string, :limit => 60, :default => ''
+    change_column :repositories, :password, :string, limit: 60, default: ''
   end
 end

And now, let’s run it again.

$ bin/rubocop db/migrate/20110226120112_change_repositories_password_limit.rb
Inspecting 1 file
.

1 file inspected, no offenses detected

We’ve finally pleased RuboCop.

By the way, we were able to use auto-correct on all of the content thanks to the —-auto-correct option.

$ bin/rubocop -a db/migrate/20110226120112_change_repositories_password_limit.rb
Inspecting 1 file
C

Offenses:

db/migrate/20110226120112_change_repositories_password_limit.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration
^
db/migrate/20110226120112_change_repositories_password_limit.rb:2:1: C: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration
^
db/migrate/20110226120112_change_repositories_password_limit.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration
^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:3:54: C: [Corrected] Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => nil, :default => ''
                                                     ^^^^^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:3:69: C: [Corrected] Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => nil, :default => ''
                                                                    ^^^^^^^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:7:54: C: [Corrected] Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => 60, :default => ''
                                                     ^^^^^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:7:68: C: [Corrected] Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => 60, :default => ''
                                                                   ^^^^^^^^^^^

1 file inspected, 7 offenses detected, 6 offenses corrected

Defining the coding standard in the file settings

By setting up the .rubocop.yml, you can disable Cops that are not important to your project, or adjust the threshold values for the metric systems.

We believe the purpose of the project is to make developing more efficient, not just following the coding conventions. So, this function allows for flexibility and is extremely useful.

Let’s return the previously mentioned source to its original, and recreate it using the rubocop.yml, as shown below. Please note that the following content may not be the most reasonable method, as it is simply a test.

Style/Documentation:
  Enabled: false

Layout/EmptyLineAfterMagicComment:
  Enabled: false

Metrics/LineLength:
  Max: 100

The results of the execution are as follows.

$ bin/rubocop  db/migrate/20110226120112_change_repositories_password_limit.rb
Inspecting 1 file
C

Offenses:

db/migrate/20110226120112_change_repositories_password_limit.rb:1:1: C: Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
class ChangeRepositoriesPasswordLimit < ActiveRecord::Migration
^
db/migrate/20110226120112_change_repositories_password_limit.rb:3:54: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => nil, :default => ''
                                                     ^^^^^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:3:69: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => nil, :default => ''
                                                                    ^^^^^^^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:7:54: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => 60, :default => ''
                                                     ^^^^^^^^^
db/migrate/20110226120112_change_repositories_password_limit.rb:7:68: C: Style/HashSyntax: Use the new Ruby 1.9 hash syntax.
    change_column :repositories, :password, :string, :limit => 60, :default => ''
                                                                   ^^^^^^^^^^^

1 file inspected, 5 offenses detected

It seems the Cop that we set is missing.

The easiest way to see what kind of setting items are available, you can see the following items on RuboCop

config/disabled.yml
config/enabled.yml

Supported check types

The fastest way would be to check rubydoc

There are many features available including layout notations, checking for bugs with Lint, security, and Rails.

To give a brief overview, the following features are available.

Layout
A Cop related to layout such as indentations
Lint
A Cop for grammatically correct but confusing formatting
Metrics
A Cop for the complexity of class and method
Naming
A Cop for the naming of method, constants, and files
Performance
A Cop for its performance
Rails
A Cop specifically for Rails description
Security
A Cop on descriptions that tend to be security holes
Style
A Cop for detailed description methods
Bundler
A Cop for bundler’s formatting such as Gemfile
Gemspec
A Cop for Gemspec descriptions

Impressions and thoughts

As mentioned above, there are a vast number of types to check. Also, the development’s level of activity is also active. However, we feel that the default settings have been set a bit rigidly, so you may need to make the necessary adjustments while using .rubocop.yml in code developing.

The auto-correct feature is extremely powerful, and the developer’s burden is reduced so that they can focus more on the code’s true nature.

Although we don’t recommend you to over-use it, if you prefer to prioritize developing code more efficiently rather than following the coding conventions, there is a way to partially disable the checking feature. The method is as follows.

# rubocop:disable Metrics/MethodLength, Metrics/AbcSize
def foo
    a looong method
end
# rubocop:enable Metrics/MethodLength, Metrics/AbcSize

Rails Best Practices

Introduction

Rewriting the Gemfile

In the group: test, add the following line.

gem ‘rails_best_practices’

Linking with SublimeText2 and Textmate2

Since I am only familiar with Vim, I have not investigated this option in detail, you can link Rails Best Practices with the following editors.

By adding handler, you can seamlessly use SublimeText2
By adding tmbundle, you can seamlessly use Textmate2

Creating databases.yml and introducing Gem

The process is the same as RuboCop.

production: &production
  adapter: sqlite3
  database: db/redmine.sqlite3

development:
  <<: *production

test:
  <<: *production

$ bundle install --path=vendor/bundle --binstubs

Testing its operation

The excerpts are shown below.

$ bin/rails_best_practices .
Source Code: |===================================================================================================================================================================|
/Users/foobar/vcswork/redmine/app/models/enumeration.rb:21 - default_scope is evil
/Users/foobar/vcswork/redmine/db/migrate/007_create_journals.rb:34 - isolate seed data
/Users/foobar/vcswork/redmine/app/controllers/account_controller.rb:61 - move model logic into model (@token use_count > 4)
/Users/foobar/vcswork/redmine/app/views/imports/settings.html.erb:9 - move code into helper (array_count >= 3)
/Users/foobar/vcswork/redmine/app/views/wiki/rename.html.erb:14 - remove trailing whitespace

Please go to https://rails-bestpractices.com to see more useful Rails Best Practices.

Found 1316 warnings.

With no settings, there seems to be a total of 1316 detected warnings. This is less than with RuboCop. However, you can see specialized messages in Rails such as move code into model or default_scope is evil.

You can refer to rails-bestpractices.com for the details of the message and how to cope with it.

As an interesting feature, if you set $ bin/rails_best_practices -f html ., you will receive an output of rails_best_practices_output.html, and it will show a list of each type of warning.

Fixing with warnings

In the above description, the main reasons for the warning are the following:

default_scope is evil means you used the default cope.
https://rails-bestpractices.com/posts/2013/06/15/default_scope-is-evil/

2. isolate seed data means you wrote the seed in migration.
https://rails-bestpractices.com/posts/2010/07/24/isolating-seed-data/

3. move model logic into model: This is the so-called fat controller.
https://rails-bestpractices.com/posts/2010/07/21/move-model-logic-into-the-model/

4. move code into helper means you haven’t escaped the view’s complex code to helper.
https://rails-bestpractices.com/posts/2010/07/24/move-code-into-helper/

5. remove trailing whitespace means there are unnecessary line spaces at the end.
https://rails-bestpractices.com/posts/2010/12/02/remove-trailing-whitespace/

Please search for “warning name rails-bestpractices” to find out more about each type of warning.

Now, let’s fix each warning.

default scope is evil

It uses the default scope from ActiveRecord. Because the solution depends on the requirements, it is not possible to solve this all at once. However, due to the following reasons, it may not be favorable to use it (please refer to the webpage mentioned above).

As long as you don’t delete it using unscope, you cannot override the default scope.
It may affect the model’s initialization.

isolate seed data

We recommend you not to tamper with the record within migration, because of the inconsistency of the record declaration. It is important to divide the schema change and input for seed data.

Specifically, we can leave it up to the db/seeeds.rb for inserting record.

$ git diff --cached
diff --git a/db/migrate/007_create_journals.rb b/db/migrate/007_create_journals.rb
index 63bdd2374..6fa28c61c 100644
--- a/db/migrate/007_create_journals.rb
+++ b/db/migrate/007_create_journals.rb
@@ -27,13 +27,6 @@ class CreateJournals < ActiveRecord::Migration

Permission.create :controller => "issues", :action => "history", :description => "label_history", :sort => 1006, :is_public => true, :mail_option => 0, :mail_enabled => 0

-    # data migration
-    IssueHistory.all.each {|h|
-      j = Journal.new(:journalized => h.issue, :user_id => h.author_id, :notes => h.notes, :created_on => h.created_on)
-      j.details << JournalDetail.new(:property => 'attr', :prop_key => 'status_id', :value => h.status_id)
-      j.save
-    }
-
     drop_table :issue_histories
   end

diff --git a/db/seeds.rb b/db/seeds.rb
new file mode 100644
index 000000000..f8f66c2f5
--- /dev/null
+++ b/db/seeds.rb
@@ -0,0 +1,8 @@
+# data migration
+IssueHistory.all.each {|h|
+  j = Journal.new(:journalized => h.issue, :user_id => h.author_id, :notes => h.notes, :created_on => h.created_on)
+  j.details << JournalDetail.new(:property => 'attr', :prop_key => 'status_id', :value => h.status_id)
+  j.save
+}

move model logic into model

Essentially, the logic that is meant to be gathered with a model is instead gathered with a controller. This is the so-called Fat Controller. The business logic should be gathered with Model as a business model.

Although it shows the lost_password of app/controllers/account_controller.rb:61 that became the warning, the actual message is long and, as the message shows, uses @token more than 4 times.

In this refactoring, it is necessary to repair not only the @token but also the @user.

def lost_password
  (redirect_to(home_url); return) unless Setting.lost_password?
  if prt = (params[:token] || session[:password_recovery_token])
@token = Token.find_token("recovery", prt.to_s)
    if @token.nil? || @token.expired?
      redirect_to home_url
      return
    end

# redirect to remove the token query parameter from the URL and add it to the session
    if request.query_parameters[:token].present?
      session[:password_recovery_token] = @token.value
      redirect_to lost_password_url
      return
    end

@user = @token.user
    unless @user && @user.active?
      redirect_to home_url
      return
    end
    if request.post?
      if @user.must_change_passwd? && @user.check_password?(params[:new_password])
        flash.now[:error] = l(:notice_new_password_must_be_different)
      else
@user.password, @user.password_confirmation = params[:new_password], params[:new_password_confirmation]
@user.must_change_passwd = false
        if @user.save
@token.destroy
          Mailer.password_updated(@user)
          flash[:notice] = l(:notice_account_password_updated)
          redirect_to signin_path
          return
        end
      end
    end
    render :template => "account/password_recovery"
    return
  else
    if request.post?
      email = params[:mail].to_s
      user = User.find_by_mail(email)
      # user not found
      unless user
        flash.now[:error] = l(:notice_account_unknown_email)
        return
      end
      unless user.active?
        handle_inactive_user(user, lost_password_path)
        return
      end
      # user cannot change its password
      unless user.change_password_allowed?
        flash.now[:error] = l(:notice_can_t_change_password)
        return
      end
      # create a new token for password recovery
      token = Token.new(:user => user, :action => "recovery")
      if token.save
        # Don't use the param to send the email
        recipent = user.mails.detect {|e| email.casecmp(e) == 0} || user.mail
        Mailer.lost_password(token, recipent).deliver
        flash[:notice] = l(:notice_account_lost_email_sent)
        redirect_to signin_path
        return
      end
    end
  end
end

move code into helper

There are a complex logic and a large code for the view layer. Because the view’s perspective will be confusing, let’s gather them towards helper.

$ git diff
diff --git a/app/helpers/imports_helper.rb b/app/helpers/imports_helper.rb
index 39f3b95ab..934b1d262 100644
--- a/app/helpers/imports_helper.rb
+++ b/app/helpers/imports_helper.rb
@@ -44,4 +44,12 @@ module ImportsHelper
       [format, f]
     end
   end
+
+  def formatting_for_options_separator(var)
+    options_for_select([[l(:label_comma_char), ','], [l(:label_semi_colon_char), ';']], var.settings['separator'])
+  end
+
+  def formatting_for_options_wrapper(var)
+    options_for_select([[l(:label_quote_char), "'"], [l(:label_double_quote_char), '"']], var.settings['wrapper'])
+  end
 end
diff --git a/app/views/imports/settings.html.erb b/app/views/imports/settings.html.erb
index 374dba5b1..c00ffbcfb 100644
--- a/app/views/imports/settings.html.erb
+++ b/app/views/imports/settings.html.erb
@@ -5,13 +5,11 @@
     <legend><%= l(:label_options) %></legend>
     <p>
       <label><%= l(:label_fields_separator) %></label>
-      <%= select_tag 'import_settings[separator]',
-            options_for_select([[l(:label_comma_char), ','], [l(:label_semi_colon_char), ';']], @import.settings['separator']) %>
+      <%= select_tag 'import_settings[separator]', formatting_for_options_separator(@import) %>
     </p>
     <p>
       <label><%= l(:label_fields_wrapper) %></label>
-      <%= select_tag 'import_settings[wrapper]',
-          options_for_select([[l(:label_quote_char), "'"], [l(:label_double_quote_char), '"']], @import.settings['wrapper']) %>
+      <%= select_tag 'import_settings[wrapper]', formatting_for_options_wrapper(@import) %>
     </p>
     <p>
       <label><%= l(:label_encoding) %></label>

Remove trailing whitespace

There are trailing whitespaces in the code.

$ git diff
diff --git a/app/views/wiki/rename.html.erb b/app/views/wiki/rename.html.erb
index ac88fd4bf..602850d13 100644
--- a/app/views/wiki/rename.html.erb
+++ b/app/views/wiki/rename.html.erb
@@ -11,7 +11,7 @@
 <p><%= f.text_field :title, :required => true, :size => 100  %></p>
 <p><%= f.check_box :redirect_existing_links %></p>
 <p><%= f.select :parent_id,
-                content_tag('option', '', :value => '') + 
+                content_tag('option', '', :value => '') +
                   wiki_page_options_for_select(
@wiki.pages.includes(:parent).to_a - @page.self_and_descendants,
@page.parent),

Other

The items listed here are merely the tip of the iceberg. It also has the ability to configure Rails, detailed helper from Mode/View/Controller, routing, and so on.

We recommend you to visit https://rails-bestpractices.com/ for the details.

Adjusting the file settings

By creating a file setting, you can freely customize the analysis content.

First, let’s create a sample model. If we run $ bin/rails_best_practices -g, then it will generate config/rails_best_practices.yml.

AddModelVirtualAttributeCheck: { }
AlwaysAddDbIndexCheck: { }
#CheckSaveReturnValueCheck: { }
............
............
MoveCodeIntoHelperCheck: { array_count: 3 }
MoveCodeIntoModelCheck: { use_count: 2 }
MoveFinderToNamedScopeCheck: { }
MoveModelLogicIntoModelCheck: { use_count: 4 }
NeedlessDeepNestingCheck: { nested_count: 2 }
............
............
RemoveUnusedMethodsInControllersCheck: { except_methods: [] }
RemoveUnusedMethodsInHelpersCheck: { except_methods: [] }
RemoveUnusedMethodsInModelsCheck: { except_methods: [] }
ReplaceComplexCreationWithFactoryMethodCheck: { attribute_assignment_count: 2 }
............
UseScopeAccessCheck: { }
UseTurboSprocketsRails3Check: { }

If you have unnecessary tests, you can cancel the respective tests by erasing the # or lines.

Also, you can change the attribute (option) values and customize Check, instead of inserting additional values to arrays.

For example, in the following line…

MoveModelLogicIntoModelCheck: { use_count: 4 }

A warning will appear if you change the use_count to 5 and you use controller’s Model with a model more than 5 times.

In addition, you can set up the ignore_files option to all Checks.

DefaultScopeIsEvilCheck: { ignored_files: ‘user.rb’ }

LongLineCheck: { max_line_length: 80, ignored_files: [‘db/migrate’, ‘config/initializers’] }

As shown in the above line, you can also get a single entry or array.

Supported checking types

Move code from Controller to Model
– Move finder to named_scope (Rails2 only)
– Use model association
– Use scope access
– Add model virtual attribute
– Replace complex creation with factory method
– Move model logic into the Model
– Check return value of “save!”
RESTful Conventions
– Overuse route customizations
– Needless deep nesting
– Not use default route
– Restrict auto-generated routes
Model
– Keep finders on their own model (rails2 only)
– The law of demeter
– Use observer
– Use query attribute
– Remove unused methods in models
– Protect mass assignment
– Destroy return value (disabled by default)
Mailer
– Use multipart/alternative as content_type of email
Migration
– Isolating seed data
– Always add database index
– Use say with time in migrations
Controller
– Use before_filter (disabled by default)
– Simplify render in controllers
– Remove unused methods in controllers
Helper
– Remove empty helpers
– Remove unused methods in helpers
View
– Move code into controller
– Move code into model
– Move code into helper
– Replace instance variable with local variable
– Simplify render in views
– Not use time_ago_in_words
Deployment
– Dry bundler in Capistrano
– Speed up assets precompilation with turbo-sprockets-rails3
Other
– Remove trailing whitespace
– Remove tab (disabled by default)
– Hash syntax (disabled by default)
– Use parentheses in method definition (disabled by default)
– Long line (disabled by default)
– Not rescue exception

Impressions and thoughts

With respect to Rails, MVC has many components such as migration, helper, and capistrano, so it quite diverse in making.

However, the development may not be as active, and the original Rails Best Practices that is its basis has stopped its updates.

That being said, the currently provided Check is also useful for the Rails5 systems.

Conclusions

Use both!

Although this is purely our personal conclusions, RuboCop is mainly for static code analysis, and rails_best_practices is a tool that places the static code analysis for Rails on top of that. In other words, the corresponding layers are different.

Even if there are overlapping tests for each layer, it would be best to introduce both tools and use it to complement each other.

The important point is not to write a code that suits the analysis tool, but instead to promptly write a code that the project needs. It would not be a correct prioritization to change the code to suit the tool’s results (by making the code harder to read, or enbugging while refactoring for example).

If you don’t have the time to consider these operation methods, it might be beneficial to consider the cloud station such as Sider.review. In Sider, you can link GitHub and Slack, inform the results of the static code analysis to the reviewer/reviewee, and easily get an environment for conducting reviews.

For more information about Sider, please go to our website.

The Types of Tools

RuboCop’s Characteristics

Rails Best Practices’s Characteristics

Trying it out

What we use

RuboCop

Introduction

Testing the operation

Responding to the offenses

Defining the coding standard in the file settings

Supported check types

Impressions and thoughts

Rails Best Practices

Introduction

Testing its operation

Fixing with warnings

Other

Adjusting the file settings

Supported checking types

Impressions and thoughts

Conclusions

Use both!

You Might Also Like

CircleCI and Sider Hosted Joint Meetup in Tokyo

How the React Developer Team Uses ESLint – Their Settings and Custom Plug-ins

Let’s Take a Look at the Rules of gas

Leave a Reply Cancel reply