hiyoko-programingの日記

プログラミングを勉強したてのひよっ子。　　　エンジニア目指して勉強中。

リファクタリング　参考資料

プログラミング・テキスト

MVCとは

皆さんがよく知っているようなWEBサービスも、最近はRuby on Railsで作られていることが多くなってきており、モデルの個数が300個を超えるような中〜大規模以上のアプリケーション開発の例が増えてきた。

WEBサービスを構築していく過程で数万行に及ぶコードとたくさんのファイルが必要になることがある。特に複数人で開発をしていると、「このページはどのファイルにコードが書かれているのか」「このデータベースの値を表示したいがロジックはどこにかけばいいのか」といったことがばらつき、次第に生産性が悪くなってしまう。

こうした問題を解決するために、Railsというフレームワークは「MVC」の考え方を採用している。MVCは「Model - View - Controller」の頭文字で、アプリケーションをそれぞれの役割に応じて以下のように3つの要素に分割する考え方。

https://tech-master.s3.amazonaws.com/uploads/curriculums//97976e1b1b9fccb2e04895499a79ba38.png

  • Controller
    • リクエストに応じて適切な処理を呼び出す
  • Model
  • View
    • 表示処理を行う

このようにそれぞれの要素は明確な役割（責務）を持っている。責務にそった処理を記述していくことで「どこに何が書いてあるのか」が明確になりコードの見通しがよくなっていく。

コードの見通しが悪くなる原因

コードが複雑になってしまう要因の一つにそれぞれのクラスにその責務外の仕事をさせてしまうことが挙げられる。

モデルの書くべき処理をコントローラに記述してしまったり、コントローラに書くべき処理をビューに記述してしまったりすることで、どこにコードが記述されているのかがわかりづらくなり、見通しが悪くなっていく。

この章ではそれぞれのクラスの責務について考えながら、適切なクラスに適切なコードを記述することでコードの見通しを改善していく具体例をみていく。

具体例を通してリファクタリングを学ぶ

いくつかの具体的なシーンに応じて、コードの見通しを改善していく。

ビュー編

ビューの構成が複雑になればなるほどビューにロジックを記述してしまうことが多くなる。
以下で紹介する方法を使って、ビューからできるだけロジックを取り除く。

ビューファイルで複雑な呼び出しを行っている場合

app/views/articles/index.html.erb
1
2
3
 
<% Article.where(status: 1).order(likes_count: :desc).limit(10).each do |article| %>
  <%= article.title %>
<% end %>

例えば、あるビューファイルに上のような記述があるとする。
この例では「表示処理を行う」ことを責務としたビュー上で、データの呼び出しに関する複雑な処理が書かれてしまっている。

このような複雑な処理は、「データ処理を行う」ことを責務としたモデルに記載する方が良い。ビューファイルに複雑な記述があると、コードの視認性が悪くなってしまうから。また、モデルに記述すると、様々なアクションで用いることができる。

また、モデルに定義した処理を行った上で、コントローラでインスタンス変数として定義してあげる。すると、ビューファイルはスッキリと記述することができる。

model
1
2
3
4
 
class Article < ActiveRecord::Base
  scope :popular, -> { order(likes_count: :desc) }
  enum status: { draft: 0, published: 1 }
end
controller
1
2
3
 
def index
  @articles = Article.published.popular.limit(10)
end
view
1
2
3
 
<% @articles.each do |article| %>
  <%= article.title %>
<% end %>

同じ処理を繰り返し記述している場合

app/views/books/show.html.erb
1
2
3
4
5
 
<% if user_signed_in? %>
  <% if current_user == @book.user %>
    <%= link_to "編集", edit_book_path(@book) %>
  <% end %>
<% end %>
app/views/comments/show.html.erb
1
2
3
4
5
 
<% if user_signed_in? %>
  <% if current_user == @comment.user %>
    <%= link_to "編集", edit_comment_path(@comment) %>
  <% end %>
<% end %>

上の２つのコードでは「ユーザーがサインインしているか」「そのインスタンスに紐付いたユーザーがログインしているユーザーと一緒であるか」を判定する処理が共通している。

この処理はアプリケーション内で使用頻度が高いものだと考えられるので、helperに処理を切り出してどのビューからでも呼び出せるようにしてあげる。

app/helpers/application_helper.rb
1
2
3
 
def current_user_has?(instance)
  user_signed_in? && current_user == instance.user
end
app/views/books/show.html.erb
1
2
3
 
<% if current_user_has?(@book) %>
   <%= link_to "編集", edit_book_path(@book) %>
<% end %>
app/views/comments/show.html.erb
1
2
3
 
<% if current_user_has?(@comment) %>
   <%= link_to "編集", edit_comment_path(@comment) %>
<% end %>

複雑な条件分岐がある場合

view
1
2
3
4
5
6
7
8
9
 
<% if @book.published? %>
  <% if @book.popular? %>
    <%=  link_to "詳細", book_path(@book) ,class: "popular" %>
  <% else %>
    <%=  link_to "詳細", book_path(@book) ,class: "normal" %>
  <% end %>
<% else %>
  <div class=normal>閲覧不可</div>
<% end %>

ビューに複雑な条件分岐がある場合もhelperに処理を移動させる。
helperではビューで使えるヘルパーメソッドが使用できる。

helper
1
2
3
4
5
6
7
8
 
def book_link(book) 
  class_name = book.popular? ? "popular" : "normal"
 if book.published?
    content_tag :a, "詳細", class: class_name
 else
   content_tag :div, "閲覧不可", class: class_name
  end
end
view
1
 
<%= book_link(@book) %>

複雑な条件分岐をhelperに移動することでビューの見通しが良くなる。
helper内は自由にrubyのコードを記述できるので、ビュー内で条件分岐をするよりもスッキリとわかりやすいコードにすることができる。

同じビューを複数回記述してしまっている場合

app/views/books/show.html.erb
1
2
3
4
5
 
<div class="book">
 <span> <%= @book.title %></span>
  <p><%= @book.description %></p>
  <span><%= @book.author %></span>
</div>
app/views/books/index.html.erb
1
2
3
4
5
6
7
8
9
 
<div class="book_list">
  <% @books.each do |book| %>
    <div class="book">
      <span> <%= book.title %></span>
      <p><%= book.description %></p>
      <span><%= book.author %></span>
    </div>
  <% end %>
</div>
繰り返し複数の箇所で使われるビューは部分テンプレートとして切り出して使用する。
app/views/books/_book.html.erb
1
2
3
4
5
 
<div class="book">
  <span> <%= book.title %></span>
  <p><%= book.description %></p>
  <span><%= book.author %></span>
</div>
app/views/books/show.html.erb
1
 
<%= render partial: "book", locals: { book: @book } %>
app/views/books/index.html.erb
1
2
3
 
<div class="book_list">
  <%= render @books %>
</div>

部分テンプレートとして切り出して使用する時の注意点は、「部分テンプレート内でインスタンス変数を使用しない」ことである。

部分テンプレート内では呼び出し元で定義されているインスタンス変数を使用することができる。しかし、これをしてしまうと呼び出し元のインスタンス変数の名前が異なる時に部分テンプレートを呼び出せなくなってしまう。

コントローラ編

ビュー同様にコントローラに複雑な処理を記述することも好ましくない。その処理が本当にコントローラに記述されなければならないのかを一度考えてみる必要がある。

複数のコントローラに同じ処理が記述されている場合(concerns)

開発の規模が大きくなるにつれ複数のコントローラに同じような処理が繰り返し記述されることがあります。この場合、

  • app/controllers/concernsにファイルを追加し、必要箇所で読み込ませる
  • 親コントローラにメソッドを定義する

などの方法でコントローラの記述をうまく共通化することができる。
まず、concernsを使った例を見てみる。

concerns ディレクトリに複数モデルで共通するコードをモジュールとして定義することでソースコードの見通しが改善できる。

app/controllers/products_controller.rb
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
 
class ProductsController < ApplicationController
  before_action :set_cart

  ~ ~ ~ 

  private

  def set_cart
    @cart = Cart.find_by(id: session[:cart_id])
    if @cart.nil?
      @cart = Cart.create
      session[:cart_id] = @cart.id
    end
  end
end
app/controllers/top_controller.rb
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
 
class TopController < ApplicationController
  before_action :set_cart

  ~ ~ ~ 

  private

  def set_cart
    @cart = Cart.find_by(id: session[:cart_id])
    if @cart.nil?
      @cart = Cart.create
      session[:cart_id] = @cart.id
    end
  end
end

上の２つのコントローラではどちらもset_cartというメソッドを定義して、インスタンス変数にカートのインスタンスを代入している。
カート情報はこの２つのコントローラ以外でも頻繁に使用する可能性があるが、そのたびにこのメソッドを追加していくのは好ましくない。そこで以下のようにconcernsに処理を切り出してあげることで同じ処理を共通化できる。

app/controllers/concerns/current_cart.rb
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
 
module CurrentCart
  extend ActiveSupport::Concern

  private

  def set_cart
    @cart = Cart.find_by(id: session[:cart_id])
    if @cart.nil?
      @cart = Cart.create
      session[:cart_id] = @cart.id
    end
  end
end
app/controllers/products_controller.rb
1
2
3
4
 
class ProductsController < ApplicationController
  include CurrentCart
  before_action :set_cart
end
app/controllers/top_controller.rb
1
2
3
4
 
class TopController < ApplicationController
  include CurrentCart
  before_action :set_cart
end

ユーザーのカート情報を扱うためにそれぞれのコントローラでこのset_cartを定義する必要があるが、定義したCurrentCartを使用したいコントローラで読み込むだけでそこに定義されているメソッドを使用することができるようになる。

複数のコントローラに同じ処理が記述されている場合(継承)

共通の処理を持つコントローラが同じ親コントローラを継承していれば、親コントローラに記述することで処理を共通化することができる。

app/controllers/sales_controller.rb
1
2
3
4
5
6
7
8
9
 
class SalesController < ApplicationController
  before_action :authorize_owner

  private

  def authorize_owner
    redirect_to root_path unless current_user.owner?
  end
end
app/controllers/customers_controller.rb
1
2
3
4
5
6
7
8
9
 
class CustomersController < ApplicationController
  before_action :authorize_owner

  private

  def authorize_owner
    redirect_to root_path unless current_user.owner?
  end
end

例えば、上の２つのコントローラではauthorize_ownerを呼び出している。この場合、SalesControllerとCustomersControllerはどちらもApplicationControllerを継承しているため、以下のように処理を共通化することができる。

app/controllers/application_controller.rb
1
2
3
4
5
6
7
8
 
def ApplicationController < ActionController::Base

  private

  def authorize_owner
    redirect_to root_path unless current_user.owner?
  end
end
app/controllers/sales_controller.rb
1
2
3
 
class SalesController < ApplicationController
  before_action :authorize_owner
end
app/controllers/customers_controller.rb
1
2
3
 
class CustomersController < ApplicationController
  before_action :authorize_owner
end

複数のアクションに同じ処理が記述されている場合

同じコントローラ内で同じような処理が繰り返し記述されている場合はcallbackを用いて共通化してあげる。

controller
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
 
def BookController < ApplicationController
  def index
    @books = Book.all
  end

  def show
    @book = Book.find(params[:id])
  end

  def new
    @book = Book.new
  end

  def create
    Book.create(book_params)
  end

  def edit
    @book = Book.find(params[:id])
  end

  def update
    @book = Book.find(params[:id])
    @book.update(book_params)
  end

  def destroy
    @book = Book.find(params[:id])
    @book.destroy
  end

  private

  ~ ~ ~

end

上のコントローラではshow, edit, update, destroyで共通して@book = Book.find(params[:id])という処理を行っている。
このような処理はbefore_actionなどのcallbackを利用して共通化してしまう。

controller
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
 
def BookController < ApplicationController
  bofore_action :set_book, only: [:show, :edit, :update, :destroy]

  def index
    @books = Book.all
  end

  def show
  end

  def new
    @book = Book.new
  end

  def create
    Book.create(book_params)
  end

  def edit
  end

  def update
    @book.update(book_params)
  end

  def destroy
    @book.destroy
  end

  private

  def set_book
     @book = Book.find(params[:id])
  end

  ~ ~ ~

end

set_bookをbefore_actionで呼び出すことによってそれぞれのアクションで何をしているのかがよりわかりやすい。

コントローラに複雑な処理を記述している場合

コントローラのコード量が多くなっている場合、本来モデルで行うべき処理がコントローラに書かれている可能性が高い。このままでは、コントローラを変更する時に影響範囲が広くなってしまい、バグが発生する可能性が高まる。

この場合には、モデルのメソッドに処理を移動する。

before
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
 
# controller
@user = User.find(params[:user_id])
@user.name = params[:user_name]
if params[company_name]
  @user.company_name = params[company_name]
  company = Company.find_by(name: params[company_name])
  if company
    @user.company = company
  end
end
@user.save
after
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
 
# controller
@user = User.find(params[:user_id])
@user.update_company(params[company_name])

# model
class User < ActiveRecord::Base
  def update_company(company_name)
    if company_name
      self.company_name = company_name
      company = Company.find_by(name: params[company_name])
      if company
        self.company = company
      end
    end
    save
  end
end

こうすることで、他のアクションでもupdate_companyを呼び出せるようになり、アプリケーション全体で処理を統一することができる。

モデル編

ビューやコントローラから呼び出される様々な処理はモデルに集約されていくために、特にモデルは肥大化し易い傾向にある。そこで、モデルに書かれた処理を幾つかの観点で切り分けていくことでモデルの見通しを改善する方法がある。

Decorator

app/models/user.rb
1
2
3
4
5
6
7
8
9
 
class User < ActiveRecord::Base
  def full_name
    "#{family_name} #{first_name}"
  end

def full_name_kana
    "#{family_name_kana} #{first_name_kana}"
  end
end

Decorator（デコレーター）とはビューとモデルの中間に位置し、モデルやビューなどに実装されやすい表示ロジックやフォーマットなどの責務を引き受けるクラス。

意識せずにコーディングをしているとモデルにビューでしか使用しないメソッドが増えていくことがある。上のfull_nameやfull_name_kanaと言ったメソッドがその例。こうしたメソッドをデコレーターに移動することでコードの見通しが改善される。

Railsでデコレーターを使用する場合にはdraperやactive_decoratorと言ったgemを使う方法が一般的。今回はより人気の高いdraperを使った例を以下に記述する。
gemの導入の仕方や細かい使い方に関してはgithubなどを参考する。

app/decorators/user_decorator.rb
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
 
class UserDecorator < Draper::Decorator
  delegate_all

  def full_name
    "#{family_name} #{first_name}"
  end

  def full_name_kana
    "#{family_name_kana} #{first_name_kana}"
  end

end
app/controllers/users_controller.rb
1
2
3
4
5
 
class UsersController < ApplicationController
  def show
    @user = User.find(params[:id]).decorate
  end
end
app/views/users/show.html.erb
1
2
 
<%= @user.full_name %>
<%= @user.full_name_kana %>

Validator

app/models/article.rb
1
2
3
 
class Article < ActiveRecord::Base
  validates :url, format: { with: /\A#{URI::regexp(%w(http https))}\z/ }
end
app/models/products.rb
1
2
3
 
class Article < ActiveRecord::Base
  validates :url, format: { with: /\A#{URI::regexp(%w(http https))}\z/ }
end

モデルの役割の一つにバリデーションがある。バリデーションとはデータの整合性を保つために、データを検証する機能のこと。

あるモデルのバリデーションに複雑な処理があったり、複数のモデルに共通のバリデーションが存在する場合にはそれらをモデルから切り離すことでリファクタリングが可能。

app/models/articles.rb
1
2
3
 
class Article < ActiveRecord::Base
  validates :name, url_format: true
end
app/models/products.rb
1
2
3
 
class Article < ActiveRecord::Base
  validates :url, url_format: true
end
app/validators/url_format_validator.rb
1
2
3
4
5
6
7
 
class UrlFormatValidator < ActiveModel::EachValidator
  def validate_each(record, attribute, value)
    if value.present? && value !~ /\A#{URI::regexp(%w(http https))}\z/
      record.errors[attribute] << "のフォーマットが不正です"
    end
  end
end

上の例ではActiveModel::EachValidatorを継承したクラスの中にvalidate_eachというメソッドを定義している。このクラスのファイル名から_validatorを取り除いたものを各クラスのvalidatesメソッドに引数として渡すと、そのカラムを検証する際にvalidate_eachメソッドが実行される。

また、様々な属性値に対して複雑な検証を行う場合などには以下のような方法もある。

app/models/event.rb
1
2
3
 
class event < ActiveRecord::Base
  validates_with RangeValidator
end
app/validators/range_validator.rb
1
2
3
4
5
6
7
 
class RangeValidator < ActiveModel::Validator
  def validate(record)
    unless start_time < finish_time
      record.errors.add :base, "finish_timeはstart_timeよりも後に設定してください。"
    end
  end
end

Callback

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
 
class BankAccount < ActiveRecord::Base
  before_save      EncryptionWrapper.new
  after_save       EncryptionWrapper.new
  after_initialize EncryptionWrapper.new
end

class EncryptionWrapper

  def before_save(record)
    record.credit_card_number =  encrypt(record.credit_card_number)
  end

  def after_save(record)
    record.credit_card_number = decypt(record.credit_card_number)
  end

  def after_initialize(record)
    record.credit_card_number = decypt(record.credit_card_number)
  end

  private

    def encrypt(value)
      # 暗号化の処理
    end

    def decrypt(value)
      # 解読の処理
    end
end

コントローラ同様にモデルにもvalidationの直前に実行されるbefore_validationであったり、saveの直後に実行されるafter_saveなど様々なタイミングで実行されるコールバックが存在する。

開発が大規模になるとコールバックにたくさんのメソッドが登録され、メソッド同士の関係性がわかりづらくなっていくことがある。

そのようなときには、callbackの引数に、そのコールバックと同名のメソッドを持つインスタンスを渡すことで、コールバックの処理を別のクラスに移動することができる。

この例ではbefore_saveでクレジットカードナンバーを暗号化し、after_initialize, after_saveで解読を行っている。このようにコールバック同士に関係性がある場合には別の一つのクラスとして扱うことによって関係性を明確にすることができる。