rails_best_practicesの指摘を修正してみた

　今回はrails_best_practicesの指摘を修正してみる。 survey!に掛けてみたところ23個の警告が出た。

インストール

　developmentグループのところにgemを追加してbundle install。

Gemfile

gem 'rails_best_practices'

実行

　今回はhtmlに出力させたいので以下で実行。

bundle exec rails_best_practices -f html .

警告一覧

Filename	Line Number	Warning Message
app/views/survey/_result_question.html.slim	14	move code into model (q use_count > 2)
app/views/kaminari/_paginator.html.slim	6	move code into model (page use_count > 2)
config/routes.rb	7	needless deep nesting (nested_count > 2)
app/helpers/answer_helper.rb	1	remove empty helpers
app/helpers/question_helper.rb	1	remove empty helpers
app/helpers/survey_helper.rb	1	remove empty helpers
app/helpers/welcome_helper.rb	1	remove empty helpers
app/models/free_choice.rb	2	remove unused methods (FreeChoice#value_data)
app/models/surveys.rb	5	remove unused methods (Surveys#my_collab)
app/models/multiple_choice.rb	10	remove unused methods (MultipleChoice#value_data)
app/models/date_choice.rb	12	remove unused methods (DateChoice#value_data)
app/models/question.rb	21	remove unused methods (Question#resize_image_url)
app/models/single_choice.rb	12	remove unused methods (SingleChoice#value_data)
app/views/question/_single_choice.html.slim	1	replace instance variable with local variable
app/views/question/_single_choice.html.slim	3	replace instance variable with local variable
app/views/question/_multiple_choice.html.slim	1	replace instance variable with local variable
app/views/question/_multiple_choice.html.slim	4	replace instance variable with local variable
app/views/question/_date_choice.html.slim	1	replace instance variable with local variable
app/views/question/_date_choice.html.slim	8	replace instance variable with local variable
app/views/question/_free_choice.html.slim	2	replace instance variable with local variable
app/controllers/survey_controller.rb	46	simplify render in controllers
app/controllers/survey_controller.rb	62	simplify render in controllers
app/views/welcome/index.html.slim	13	remove trailing whitespace

move code into model (q use_count > 2)

モデルに条件を寄せたほうが良いということ。 今回はdecorator側へ移動。 Kaminari Themesのテンプレートにも指摘があったが今回はexclude指定することにした。

before

app/views/survey/_result_question.html.slim

- if q.single_choice? || q.multiple_choice? || q.date_choice?

after

app/decorators/question_decorator.rb

def choice?
  single_choice? || multiple_choice? || date_choice?
end

app/views/survey/_result_question.html.slim

- if q.choice?

needless deep nesting (nested_count > 2)

　ルーティングのネストが3つになっているのがまずいらしい。 ここで言うと答え（answer）は質問（question）に属しており、アンケート（survey）に属しているわけではないということらしい。

before

config/routes.rb

resources :survey do
  resources :question, only: :show do
    resources :answer, only: [:create, :update]
  end
end

after

config/routes.rb

resources :survey do
  resources :question, only: :show
end
resources :question, only: :show do
  resources :answer, only: [:create, :update]
end

　確かに書いている時にも3つはやり過ぎかなとは思ったが・・・。 しかし、これを変えると基点テーブルが変わるのでActiveRecordの書き方も変えないといけない。

remove empty helpers

空のヘルパーはロード時間もかかるし勿体無いということだ。 指摘の内容のソースを実行して削除。

Dir.glob("app/helpers/**/*.rb").each do |file|
  if !File.read(file).index('def')
    FileUtils.rm file
    FileUtils.rm_f file.sub("app/", "test/unit/").sub(".rb", "_test.rb") if File.exists?("test")
    FileUtils.rm_f file.sub("app/", "spec/").sub(".rb", "_spec.rb") if File.exist?("spec")
  end
end

remove unused methods

　タイトルの通り使われていないメソッドの削除。

replace instance variable with local variable

　partialはインスタンス変数を参照せずに、localで変数を渡したものだけにした方が良い。 明示的に指定することで何が必要かを明確に出来るメリットが有る。

simplify render in controllers

　renderを簡潔に書けということ。

before

app/controllers/survey_controller.rb

render action: 'edit'

after

app/controllers/survey_controller.rb

render :edit

remove trailing whitespace

　不要な行末の空白を削除しろということ。 テーブルで空のヘッダがあったのを以下のように記述していたのでパイプ部分を削除。

before

th.two.wide.computer.only
  | 

after

th.two.wide.computer.only

結果

まとめ

　ベストプラクティスがツールでチェック出来るのは便利で、Railsを学ぶ上では非常に良いと感じた。 そして知っていたとしてもチェックの意味もあるのでお勧めだ。