Что было бы лучшим способом закодировать это, если предложение else?

Question

У меня есть следующий код. Я все еще новичок в 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 равен нулю.

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

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 

корень

Answer 1

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

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

Выходы NilClass

Answer 2

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, так что это не может реально составить, но вы получите идею.

Answer 3

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

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]}) 

конец

Answer 4

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

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.