1

プライベートメソッドでインスタンス変数の初期化を隠すのは良い習慣ですか?

たとえば、いくつかのアクションを持つユーザー コントローラーがあります。

class UsersController < ApplicationController
  before_filter :get_user, only: [:show, :edit, :update, :destroy]
  before_filter :set_user, only: [:new, :create]

  def index
    @users = User.all
  end

  def show
  end

  def new
  end

  def edit
  end

  def create
    if @user.save
      redirect_to @user, notice: 'User was successfully created.'
    else
      render action: 'new'
    end
  end

  def update
    if @user.update_attributes(params[:user])
      redirect_to @user, notice: 'User was successfully updated.'
    else
      render action: 'edit'
    end
  end

  def destroy
    @user.destroy
    redirect_to users_path
  end

private

  def get_user
    @user = User.find(params[:id])
  end

  def set_user
    @user = User.new(params[:user])
  end
end

魔法のようだと言う人もいますが、DRY です。どう思いますか？

4

2 に答える 2

0

これは私にはあまりにも乾燥しています。

before_filterインスタンス変数の初期化などの日常的なものについては、メソッドが空白に見えるにもかかわらず、何かが起こっているため、私は気が狂います。メソッドが空の場合は大したことではありませんが、メソッドが大きいとフィルターがわかりにくくなったり、完全に見落としたりする可能性があります。次に、フィルター メソッドを探し出し、頭の中でワークフローを再構築する必要があります。メンテナンスが必要以上に難しくなります。

フィルターを使用せずに、getter/setter メソッドを適切に呼び出します。

def show
  get_user
end

そうすれば、初期化が行われている場所を確認できます。どうしてもフィルターを使用する場合は、メソッドにコメントを入れて、フィルターが適用されていることを伝えます。

個人的にはbefore_filter、条件付きロジックのみを予約しています。

于 2013-01-19T12:16:06.873 に答える
0

それらは隠されているのではなく、すぐそこにあります。

個人的には、DRY に関しては、従うのが好きなルールがあります (どこかで読んだことがありますが、どこで読んだか覚えていません。許してください)。しかし、もう一度複製したい場合は、それを 1 か所に抽出します。

あなたの:load_user例は問題ありませんが、気にしません:set_user。

于 2013-01-19T10:51:47.750 に答える