LoginSignup
3
5

More than 3 years have passed since last update.

MVCのリファクタリング

Last updated at Posted at 2020-04-06

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

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

【例】

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

この例では「表示処理を行う」ことを責務としたビュー上で、データの呼び出しに関する複雑な処理が書かれています。
このような複雑な処理は、「データ処理を行う」ことを責務としたモデルに記載します。なぜかというとビューファイルに複雑な記述があると、コードの視認性が悪くなります。また、モデルに記述すると、様々なアクションで用いることができます。
また、モデルに定義した処理を行った上で、コントローラでインスタンス変数として定義します。
model

class Article < ActiveRecord::Base
  scope :popular, -> { order(likes_count: :desc) }
  enum status: { draft: 0, published: 1 }
end

controller

def index
  @articles = Article.published.popular.limit(10)
end

view

<% @articles.each do |article| %>
  <%= article.title %>
<% end %>

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

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

上記の2つのコードでは「ユーザーがサインインしているか」「そのインスタンスに紐付いたユーザーがログインしているユーザーと一緒であるか」を判定する処理が共通しています。
この処理はアプリケーション内で使用頻度が高いものだと考えられるので、helperに処理を切り出してどのビューからでも呼び出せるようにします。

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

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

view

<% 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

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

<%= book_link(@book) %>

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

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

app/views/books/show.html.erb
<div class="book">
 <span> <%= @book.title %></span>
  <p><%= @book.description %></p>
  <span><%= @book.author %></span>
</div>
app/views/books/index.html.erb
<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
<div class="book">
  <span> <%= book.title %></span>
  <p><%= book.description %></p>
  <span><%= book.author %></span>
</div>
app/views/books/show.html.erb
<%= render partial: "book", locals: { book: @book } %>
app/views/books/index.html.erb
<div class="book_list">
  <%= render @books %>
</div>

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

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

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

開発の規模が大きくなるにつれ複数のコントローラに同じような処理が繰り返し記述されることがあります。この場合、
app/controllers/concernsにファイルを追加し、必要箇所で読み込ませます。
親コントローラにメソッドを定義する
などの方法でコントローラの記述を共通化することができます。
concerns ディレクトリに複数モデルで共通するコードをモジュールとして定義することでソースコードの見通しが改善できます。
【例】

app/controllers/products_controller.rb
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
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

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

app/controllers/concerns/current_cart.rb
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
class ProductsController < ApplicationController
  include CurrentCart
  before_action :set_cart
end
app/controllers/top_controller.rb
class TopController < ApplicationController
  include CurrentCart
  before_action :set_cart
end

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

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

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

app/controllers/sales_controller.rb
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
class CustomersController < ApplicationController
  before_action :authorize_owner

  private

  def authorize_owner
    redirect_to root_path unless current_user.owner?
  end
end

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

app/controllers/application_controller.rb
def ApplicationController < ActionController::Base

  private

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

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

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

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

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

# 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

# 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
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を使った例を記述します。
active_decorator Github リポジトリ
「デコレーター」を導入するためのgemです。
【例】

app/decorators/user_decorator.rb
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
class UsersController < ApplicationController
  def show
    @user = User.find(params[:id]).decorate
  end
end
app/views/users/show.html.erb
<%= @user.full_name %>
<%= @user.full_name_kana %>

Validator(バリデーター)

【例】

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

モデルの役割の一つにバリデーションがあります。バリデーションとはデータの整合性を保つために、データを検証する機能のことです。
あるモデルのバリデーションに複雑な処理があったり、複数のモデルに共通のバリデーションが存在する場合にはそれらをモデルから切り離すことでリファクタリングが可能になります。
【例】

app/models/articles.rb
class Article < ActiveRecord::Base
  validates :name, url_format: true
end
app/models/products.rb
class Article < ActiveRecord::Base
  validates :url, url_format: true
end
app/validators/url_format_validator.rb
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
class event < ActiveRecord::Base
  validates_with RangeValidator
end
app/validators/range_validator.rb
class RangeValidator < ActiveModel::Validator
  def validate(record)
    unless start_time < finish_time
      record.errors.add :base, "finish_timeはstart_timeよりも後に設定してください。"
    end
  end
end

Callback(コールバック)

【例】

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で解読を行っています。このようにコールバック同士に関係性がある場合には別の一つのクラスとして扱うことによって関係性を明確にすることができます。

3
5
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
3
5