2017-04-29

モデルのロジックをモデルに移動する

翻訳

Bad Smell

class PostsController < ApplicationController
  def publish
    @post = Post.find(params[:id])
    @post.update_attribute(:is_published, true)
    @post.approved_by = current_user
    if @post.created_at > Time.now - 7.days
      @post.popular = 100
    else
      @post.popular = 0
    end

    redirect_to post_url(@post)
  end
end

上記の例では、PostsControllerはpostを公開したいので、まずpostを検索し、 postのis_published属性とapproved*1に値をセットし、 popular属性にcreated_atの値をもとに値を設定してと、 PostsControllerはロジックを知りすぎており、コントローラーの責務ではなく、モデルが操作するべきです。

Refactor

class Post < ActiveRecord::Base
  def publish
    self.is_published = true
    self.approved_by = current_user
    if self.created_at > Time.now - 7.days
      self.popular = 100
    else
      self.popular = 0
    end
  end
end

class PostsController < ApplicationController
  def publish
    @post = Post.find(params[:id])
    @post.publish

    redirect_to post_url(@post)
  end
end

公開ロジックをコントローラーからモデルに移動してpublishメソッドをPostモデルを作ったので、コントローラーからpublishメソッドを呼ぶだけでよくなりました。これで綺麗になりました。

感想

モデル内でcurrent_userを使っているのが気になるなぁとおもった。

*1:approved_byの誤り？

2017-04-28

モデルに仮想的な属性を追加する

Rails Best Practiceを読む

元記事はこちら

翻訳

Bad Smell

<% form_for @user do |f| %>
  <%= text_field_tag :full_name %>
<% end %>

class UsersController < ApplicationController
  def create
    @user = User.new(params[:user])
    @user.first_name = params([:full_name]).split(' ', 2).first
    @user.last_name = params([:full_name]).split(' ', 2).last
    @user.save
  end
end

上記の例では、full_name入力欄がある画面ですが、データベースには姓、名を分けて保存したいので、コントローラーで姓と名を分割して@userオブジェクトに設定しています。しかし、モデルに属性を設定するのは、コントローラーの責務ではありませんから、仮想的な属性により、この処理をモデルにさせるべきです。

Refactor

class User < ActiveRecord::Base
  def full_name
    [first_name, last_name].join(' ')
  end

  def full_name=(name)
    split = name.split(' ', 2)
    self.first_name = split.first
    self.last_name = split.last
  end
end

<% form_for @user do |f| %>
  <%= f.text_field :full_name %>
<% end %>

class UsersController < ApplicationController
  def create
    @user = User.create(params[:user])
  end
end

こうすることで、Userモデルにfull_name=という仮想的な属性が付加され、姓と名を設定するするようになりました。こうすれば、たった１行でuserを作成することができるようになり、Userモデルの中で姓と名を分割できるようになるので、いい感じになります。

感想

分割して入力された電話番号をつなげてDBに放り込みたいとか、例とは逆の方向でなら使う機会は多そう。 Railsガイドによると、confirmation: true な時にも仮想属性が生まれてるみたい。

Active Record バリデーション | Rails ガイド

2017-04-26

スコープを使ってアクセスする

Rails Best Practiceを読む

元記事はこちら

翻訳

オブジェクトのオーナーとcurrent_userを比較することで、アクセス権をチェックしたいとおもったら、そんな冗長でイケてない書き方はせずに、スコープをつかてアクセスしましょう。

Bad Smell

class PostsController < ApplicationController
  def edit
    @post = Post.find(params[:id])
    if @post.user != current_user
      flash[:warning] = 'Access denied'
      redirect_to posts_url
    end
  end
end

上記の例では、postのuserをcurrent_userと比較して、一致しなかった場合に、postの編集を許可しないようにしている。しかし、このやり方は、edit,update,destroyなどを実行できるかチェックするには冗長すぎる。スコープを使ってリファクタリングしよう。

Refactor

class PostsController < ApplicationController
  def edit
    # raise RecordNotFound exception (404 error) if not found
    @post = current_user.posts.find(params[:id])
  end
end

こうすることで、current_user.postsにだけ紐づくpost、言い換えるとcurrent_userに紐づくことが保証されたpostを検索することができた。紐付いたpostが存在しない場合、404エラーが返却される。 current_userとオーナーを比較する必要は全くなく、スコープアクセスを使うだけでアクセス権のチェックをより単純に書くことができる。

感想

404が返るってところは、厳密に言うとActiveRecord::RecordNotFoundがraiseされて、rescue_fromを書いていれば、ActionController::Rescueがひろう仕組みらしい。

2017-04-24

Modelの関連付けを利用する

Rails Best Practiceを読む

元記事はこちら

翻訳

Bad Smell

class PostsController < ApplicationController
  def create
    @post = Post.new(params[:post])
    @post.user_id = current_user.id
    @post.save
  end
end

この例では、user_idは明らかに@postに割り当てられます。大きな問題にはなりませんがModelの関連付けを利用すれば、この行は書かずに済みます。

Refactor

class PostsController < ApplicationController
  def create
    @post = current_user.posts.build(params[:post])
    @post.save
  end
end

class User < ActiveRecord::Base
  has_many :posts
end

userとpostの間に１対多の関連付けを定義したので、current_user.posts.build または current_user.posts.create によってpostを生成することができますし、current_userのidはactiverecordによって自動的にpostのuser_id属性に割り当てられます。

感想

動かすには

class Post < ActiveRecord::Base
  belongs_to :user
end

が必要かな。

正直このへんはよくわかっていないので整理。

アソシエーション	関連	特徴
belongs_to	1:1	外部キーを持つ方のモデルに設定する。
has_one	1:1	外部キーを持たれる方のモデルに設定する。
has_many	1:n	外部キーを持たれる方のモデルに設定する。
has_many :through	n:n	`:through`以下には中間テーブルの名前が入る
has_one :through	1:1	`:through`以下には中間テーブルの名前が入る
has_and_belongs_to_many	n:n	中間テーブルのmodelを持たない。(DBにテーブルは必要)

リレーションシップのモデルそれ自体を独立したエンティティとして扱いたい(両モデルの関係そのものについて処理を行いたい)のであれば、中間に結合モデルを使用するhas_many :throughリレーションシップを選ぶのが最もシンプルです。リレーションシップのモデルで何か特別なことをする必要がまったくないのであれば、結合モデルの不要なhas_and_belongs_to_manyリレーションシップを使用するのがシンプルです

ふむ。関連テーブルをmodelから意識する必要がなければhas_and_belongs_to_manyを使うと。

2017-04-23

検索処理をスコープに入れる

Rails Best Practiceを読む

元記事はこちら

翻訳する

BadSmell

class PostsController < ApplicationController
  def index
    @published_posts = Post.find(:all, :conditions => { :state => 'published' },
                                       :limit => 10,
                                       :order => 'created_at desc')

    @draft_posts = Post.find(:all, :conditions => { :state => 'draft' },
                                   :limit => 10,
                                   :order => 'created_at desc')
  end
end

コントローラー内で公開済み(published_posts)と下書き(draft_posts)の検索をするために、込み入った検索処理が書かれているが、以下の２点がよくない。

コントローラー内に込み入った検索処理を書いてしまうと、たいていの場合長くなってしまい、読みづらい。
同じような込み入った検索処理は、他のコントローラーにも存在するだろうし、ロジックを変更させたくなった場合に、複数の箇所に手を入れないといけないという作りは、バグの温床になる。

Refactor((~3.0.9。Rails3.1~ はnamed_scope -> scope))

    class PostsController < ApplicationController
      def index
        @published_posts = Post.published
        @draft_posts = Post.draft
      end
    end
    
    class Post < ActiveRecord::Base
      named_scope :published, :conditions => { :state => 'published' },
                              :limit => 10,
                              :order => 'created_at desc'
      named_scope :draft, :conditions => { :state => 'draft' },
                              :limit => 10,
                              :order => 'created_at desc'
    end

込み入った検索処理をコントローラーからモデルに移動した。ご覧の通り、コントローラーのコードが非常にシンプルになった。込み入った検索処理はモデルにだけ存在するので、ロジックが変わったら、モデル内のコードを変更するだけでいい。

updated: Rails3以降では、named_scopeの代わりにscopeを使いましょう。

感想

scopeだとこんな感じかな

 class Post < ActiveRecord::Base
      scope :published, :conditions => { :state => 'published' },
                              :limit => 10,
                              :order => 'created_at desc'
      scope :draft, :conditions => { :state => 'draft' },
                              :limit => 10,
                              :order => 'created_at desc'
    end

limit/orderの意味上の役割にもよるけど、ここを別scopeにして、mergeしてもいいのかな。

  class PostsController < ApplicationController
      def index
        @published_posts = Post.published.view_limit
        @draft_posts = Post.draft.view_limit
      end
    end

    class Post < ActiveRecord::Base
      scope :published, -> { where(:state, "published" }
                              
      scope :draft, -> { where(:state, "draft" }
      
      scope :view_limit, -> :conditions => :limit => 10,
                              :order => 'created_at desc'
                        
    end

ぜんまい(手巻き式)ブログ

主に勉強メモ

モデルのロジックをモデルに移動する

翻訳

Bad Smell

Refactor

感想

モデルに仮想的な属性を追加する

翻訳

Bad Smell

Refactor

感想

スコープを使ってアクセスする

翻訳

Bad Smell

Refactor

感想

Modelの関連付けを利用する

翻訳

Bad Smell

Refactor

感想

検索処理をスコープに入れる

翻訳する

感想