Rails リファクタリング まとめてみた

書いてあること

• リファクタリングとは
• リファクタリングは何のため？

• ビューのリファクタリング
  1.enum
  2.enumの使い方

• コントローラーのリファクタリング
  1.scopeを使って検索ロジックをモデルに移す
  2.コントローラに書かれたロジックをモデルのメソッドで書き直す

• 参考ページ

• 終わりに

リファクタリングとは

リファクタリングは処理の内容を変えずに冗長なコードを削除したり、コードを改善すること。

リファクタリングは何のため？

• 読み手が最短時間でコードを理解できるようになる(チーム開発など、人と作業をするとき負担を減らす)
• 修正がしやすくなる（未来の自分への優しさ）

ビューのリファクタリング

1.enum

int型、boolean型で定義されたカラムを、文字列で表現できる

schema.rb

こんなテーブルができてます
  create_table "tasks", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t|
    t.string "title", null: false
    t.string "content"
    t.datetime "start_at", null: false
    t.datetime "finish_at", null: false
    t.integer "kind", null: false　←今回はこのカラムのロジックでビューの表示が変わるような記述がある
    t.boolean "finished", default: false, null: false

index.html.haml

- if task.kind == 0
      私用
- if task.kind == 1
      仕事
- if task.kind == 2
      その他

この記述で保守性が下がる理由

• kindの0, 1, 2がそれぞれ何を意味するのか、ビューを見ないと分からない
• kindの種類が増える度、新たにif文を足さなければいけない

2.enumの使い方

1.モデルにenumを定義

モデル

class Task < ApplicationRecord
  enum kind: [:individual, :work, :others]
end

2.hamlの書き換え

haml

%tr
  %td
    - if task.kind == 0
      私用
    - if task.kind == 1
      仕事
    - if task.kind == 2
      その他
  %td
　  = task.title
↓
/if文を削除し、task.kindと書き換える/
%tr
  %td
    = task.kind
  %td
    = task.title
/以下省略/

3.enumを使った書き換え
enumを定義すると、[モデル名].[カラム名の複数形] のような形で、そのカラムに設定したenumを全て表示することができる

haml

.form-wrapper
  = form_for @task, html: {class: 'form-group'} do |f|

    = f.label :kind
    = f.select :kind, [0, 1, 2], {},class: 'form-control', placeholder: 0  
/配列に数字がベタが記されている部分を変えます/
/以下略/

"f.select"に引数として渡す

.form-wrapper
  = form_for @task, html: {class: 'form-group'} do |f|

   = f.label :kind
   = f.select :kind,Task.kinds.keys, {}, class: 'form-control', placeholder: 0 
 /Task.kinds.keysのように記述することによって、enumで設定したkeyの一覧を得ることができる/

Task.kindsのなかみ
=> {"individual"=>0,"work"=>1, "others"=>2}
Keyをつけると下記のようになる
=> ["individual", "work", "others"]

4.enum_helpのインストール
enum_help」というgemを利用すると、enumの日本語化が簡単に行える

gemfile

# 末尾に追記
gem 'enum_help'

$ bundle installする

5.ja.ymlとビューを編集して日本語化

ja.yml

ja:
  activerecord:
↓
#ja:の下に記述を追加する
ja:
  enums:
    task:
      kind:
        individual: "私用"
        work: "仕事"
        others: "その他"
  activerecord:

haml

%td
    = task.kind
↓
#書換える
 %td
    = task.kind_i18n

haml

= f.label :kind
= f.select :kind,Task.kinds.keys, {}, class: 'form-control', placeholder: 0
↓
#書換える
 = f.label :kind
 = f.select :kind, Task.kinds_i18n.invert, {}, class: 'form-control', placeholder: 0

コントローラーのリファクタリング

1.scopeを使って検索ロジックをモデルに移す

index.html.haml

def index
    @task = Task.new
    @tasks = Task.where('start_at > ?', Time.zone.now).order(start_at: :asc)
  end

@tasksの定義が冗長という問題は、モデルにscopeを定義することによって解決できます。

モデルにscopeを定義

models/task.rb

class Task < ApplicationRecord
  enum kind: [:individual, :work, :others]
end
↓
class Task < ApplicationRecord
  enum kind: { individual: 0, work: 1, others: 2 }

  scope :incoming,  -> { where('start_at > ?', Time.zone.now) }
end

定義したscopeを使って@tasksを再定義

controller

  def index
    @task = Task.new
    # モデルに定義したscopeはメソッドのように呼び出せる
    @tasks = Task.incoming.order(start_at: :asc)
  end

2.コントローラに書かれたロジックをモデルのメソッドで書き直す

コントローラのupdateアクション内の「finishedカラムをtrueにする処理」を、Taskモデルのインスタンスメソッドとしてmodels/task.rbに定義し直す

models/task.rb

class Task < ApplicationRecord
  enum kind: { individual: 0, work: 1, others: 2 }

  scope :incoming,  -> { where('start_at > ?', Time.zone.now) }

  # finishedカラムをtrueに更新するメソッドを定義
  def update_finished_true
    self.finished = true if self.finished == false
    self.save
  end
end

controller

  def update
    @task = Task.find(params[:id])
    if @task.finished == false
      @task.finished = true
      @task.save
      redirect_to tasks_path
    else
      render :index, alert: '既にタスク「#{task.title}」は完了しています '
    end
  end
↓
# 定義しておいたメソッドを呼び出す
  def update
    @task = Task.find(params[:id])
    @task.update_finished_true
    redirect_to tasks_path
  end

元々は「finishedがtrueか、そうでないか」で更新を行うかどうかを決定していましたが、「現状finishedがtrueのtaskについて、updateアクションを呼び出す導線がビューにないこと」 「元々値がtrueのカラムにtrueを入れても問題がないこと」を理由に、if文を削除しています。

参考ページ

復習用:修正済

Rails Webook

終わりに

リファクタリングって書き直すの理解しずらいよね。書いてはみたものの色々ぶっ飛んでてわかりずらいので知識が深まったタイミングで書き直したいな。
でも、綺麗にかければ未来の自分がわかりやすい〜ってなるのかな。いや、なってくれないと困る。笑