값의 범위가있는 리팩토링 조건

Question

Ruby on Rails 코드의 리팩터링 방법은 무엇입니까?

def select_plan 
     unless params[:plan] && (params[:plan] == '1' || params[:plan] == '2' || params[:plan] == '3' || params[:plan] == '4' || params[:plan] == '5' || params[:plan] == '6' || params[:plan] == '7' || params[:plan] == '8') 
      flash[:notice] = "Please select a membership plan to register." 
      redirect_to root_url 
     end 
    end 

Answer 1

같은 다음 뷰에 대한 전체 플랜 세부 사항에 액세스 할 수 있으므로 이름, 설명, 가격 등을 표시 할 수 있습니다. 계획의 존재는 한 장소 (예 : 데이터베이스)에만 저장됩니다. 당신이 @plan를 필요로하지 않는 경우에 당신이 말할 수 :

if(!Plan.where(:id => params[:plan]).exists?) 
    ... 
end 

중요한 점은 정확히 하나 개의 유효한 계획이 무엇인지 알고있는 것은 당신이 계획에 대해 알아야 할 모든 시간이 있어야한다는 것입니다, 당신은 부탁드립니다 한 가지는 오직 한 가지뿐입니다.

또한 유효한 계획 얻기 위해 (대신 문자 숫자 팔을 통해 하나의) 데이터베이스를 사용하는 것이 select_plan를 호출 끝 뷰 :

<% Plan.order(...).each do |p| %> 
    whatever you need to display the plan as an option... 
<% end %> 

데이터베이스에 새로운 계획을 추가하고 모든 여전히 작동 . 계획을 제거/비활성화하면 모든 것이 여전히 작동합니다. 소프트웨어 유지 관리가 쉽고, 버그가 적으며, 이해하기 쉽고, 나쁜 소프트웨어가 아닌 새로운 좋은 습관을 얻게됩니다.

Answer 2

나는 이렇게 할 것입니다. Plan 진짜 일 확인 :

def select_plan 
    unless params[:plan].in?('1'..'8') 
    flash[:notice] = "Please select a membership plan to register." 
    redirect_to root_url 
    end 
end 

또는 무 같은

는 제안 너무 짧습니다. 그것은 데이터베이스 모델 또는 단지 작은 루비 클래스 수 있습니다 : 이제

@plan = Plan.find_by(:id => params[:plan]) 
if(!@plan) 
    flash[:notice] = "Please select a membership plan to register." 
    redirect_to root_url 
end 

: 계획 번호가 데이터베이스에 다음 간단하게 말할 수있는 Plan 모델이있는 경우

# in app/models/plan.rb 
require 'set' 
class Plan 
    VALID_PLANS = Set.new('1'..'8').freeze 

    def self.valid_plan?(plan) 
    VALID_PLANS.include?(plan) 
    end 
end 

# used like 
def select_plan 
    unless Plan.valid_plan?(params[:plan]) 
    flash[:notice] = "Please select a membership plan to register." 
    redirect_to root_url 
    end 
end