Что может быть лучше, чтобы закодировать это, если тогда еще условие? - PullRequest
1 голос
/ 16 сентября 2010

У меня есть следующий код.Я все еще новичок в Ruby on Rails.Как видите, я повторяюсь 4 раза.

Я пробовал что-то вроде этого:

if @property.nil? || @property.status_id == 144 || (@property.status_id <= 16 && current_user.nil?) || (@property.status_id <= 16 && current_user.id != @property.user_id)

Но это дает мне много ошибок в случае, если @property равен нулю.Потому что тогда @ property.status_id не может быть вызван, так как @property равен nil.

В любом случае, я думаю, что опытный кодировщик Ruby on Rails понял эту идею.

  def show
    @property = Property.find(params[:id]) rescue nil
    if @property.nil?
      flash[:error]=t("The_property_was_not_found")
      redirect_to root_path
      return
    end
    if @property.status_id == 144
      flash[:error]=t("The_property_was_not_found")
      redirect_to root_path
      return
    end
    if @property.status_id <= 16 && current_user.nil?
      flash[:error]=t("The_property_was_not_found")
      redirect_to root_path
      return
    end
    if @property.status_id <= 16 && current_user.id != @property.user_id
      flash[:error]=t("The_property_was_not_found")
      redirect_to root_path
      return
    end
    @images = Image.find(:all, :conditions =>{:property_id => params[:id]})
  end

root

Ответы [ 4 ]

2 голосов
/ 16 сентября 2010
def show
    @property = Property.find(params[:id]) rescue nil
    if @property.nil? || @property.status_id == 144 || (@property.status_id <= 16 && (current_user.nil? || current_user.id != @property.user_id))
      flash[:error]=t("The_property_was_not_found")
      redirect_to root_path
    else
      @images = Image.find(:all, :conditions =>{:property_id => params[:id]})
    end
  end

Я не знаком с синтаксисом Ruby, поэтому он может не скомпилироваться, но вы поняли.

1 голос
/ 16 сентября 2010

Я думаю, что вам следует подойти к этому, определив логику «можно показать» в виде простого вспомогательного метода, который вы можете вызвать для определения, а не для того, чтобы засорять ваш метод показа всеми видами ветвей, которые в конечном итоге приводят к тому же действию.1001 *

def can_show_property?(property)
  return false unless (property)

  return false if (property.status_id == 144 or property.status_id > 16)

  return false unless (current_user && current_user.id == property.user_id)

  true
end

def show
  @property = Property.find(params[:id]) rescue nil

  unless (can_show_property?(@property))
    flash[:error]=t("The_property_was_not_found")
    redirect_to root_path
    return
  end

  @images = Image.find(:all, :conditions =>{ :property_id => params[:id] })
end

Наличие «магических» чисел в вашем коде, таких как 144, приводит к тому, что они не задаются постоянным.Обычно это намного легче понять, когда четко обозначено MyApp::PROPERTY_LOCKED.

1 голос
/ 16 сентября 2010

Это действительно точный код? || короткое замыкание и значение nil не должно быть проблемой.

@property=nil
if @property.nil? || @property.status_id == 144
   puts @property.class.to_s
end

Выходы NilClass

0 голосов
/ 16 сентября 2010

Вы можете попробовать это:

def show
begin 
  @property = Property.find(params[:id])
  if [144,16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,1,0].include?(@property.status_id)
    flash[:error]=t("The_property_was_not_found")
    if current_user && (current_user.id != @property.user_id)
      redirect_to myimmonatie_path 
    else
      redirect_to root_path 
    end
rescue
  flash[:error]=t("The_property_was_not_found")
  redirect_to root_path
end
@images = Image.find(:all, :conditions =>{:property_id => params[:id]})

конец

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...