1

行以下が長1.すぎ5.ます（私の理解では、80文字が推奨されるカットオフです）。このコードを書くための最良の方法は何でしょうか。具体的には、条件の構造-いくつかの代替コードフォーマットオプションは何ですか？

if @user.authenticate(params[:current_password]) && @user.update_attributes(params[:user])
  sign_in @user
  redirect_to @user, notice:["Profile update successful."]
else
  flash.now[:error] = @user.errors.full_messages unless @user.errors.full_messages.empty?
  render :edit
end

ありがとう！

アップデート

それが役に立ったら、私のコードを無視してください。私は主に条件付き構造のオプションとそのフォーマットに興味があります。

4

4 に答える 4

2

&&を使用する場合||、条件を複数の行に分割できます。

if @user.authenticate(params[:current_password]) && 
   @user.update_attributes(params[:user])
     # I line up the conditionals
     # and indent the code after the conditionals further
     # just for clarity
else
  # ...

しかし、80文字を超える行を折り返さないテキストエディタを持っている人を見つけた場合、私のアドバイスは、彼らの決定に責任を負うか受け入れるように指示することです。

于 2012-10-19T11:57:22.527 に答える
2

5行目は完全に削除できます。レンダリング時にフラッシュを使用する必要はありません。リダイレクトするときにのみ必要です。認証のために、を設定することをお勧めしますbefore_filter。これを使用すると、コードは次のようになります。

class UsersController < ApplicationController
  before_filter :require_logged_in_user, :only => [:edit, :update]

  # Note: @user is set in require_logged_in_user
  def update
    if @user.update_attributes(params[:user])
      sign_in @user
      redirect_to @user, notice: "Profile update successful."
    else
      render :edit
    end
  end

  private

  def require_logged_in_user
    @user = User.find(params[:id])
    redirect_to '/login' unless @user.authenticate(params[:current_password])
  end
end
于 2012-10-19T10:23:04.553 に答える
2

IMOを別々に処理する必要がある2つの条件をマージしています（クリーンな条件分岐を維持することは非常に重要です）。私は書くだろう：

if !@user.authenticate(params[:current_password]) 
  flash[:error] = "Authentication failed"
  render :edit
elsif !@user.update_attributes(params[:user])
  # are you sure about this one? Rails helpers should show these errors.
  flash[:error] = @user.errors.full_messages.to_sentence
  render :edit
else
  sign_in @user
  redirect_to @user, notice: "Profile update successful"
end
于 2012-10-19T11:40:19.973 に答える
1

関して ：

flash.now[:error] = @user.errors.full_messages unless @user.errors.full_messages.empty?

full_messagesが空であるかどうかを確認する必要はありません。空の配列を渡すときに、フラッシュがレンダリングされないようにする必要があります。

しかし、個人的には、「強打された」方法のみを使用して、それらを救出しようとします。

begin
  @user.authenticate!(params[:current_password])
  @user.update_attributes!(params[:user])
  sign_in @user
  redirect_to @user, notice: "Profile update successful"
rescue ActiveRecord::RecordInvalid => e # for example
  flash.now[:error] = @user.errors.full_messages
  render :edit
end

しかし、それは単なる個人的な好みかもしれません。

于 2012-10-19T10:22:13.550 に答える