루비 리팩토링

Question

나는 약간의 날짜를 저장하고 기본적인 조건부 검사를 수행하는 간단한 레일 사이트를 만들고있다. 나는 아래에 몇 가지 방법을 적어서 더 효율적으로 만들 수 있다고 들었다. 나는 머리를 긁적이고 어떻게해야할지 모르겠다. 내가 entry.find 글로벌해야합니까? 아니면 더 확실한 해결책이 있습니까? name을 위해 : 당신이 액세스를 할 필요가 없습니다

def name 
    @fname = params[:fst_name] 
    @lname = params[:lst_name] 
    @entry = Entry.create({:first_name => @fname, :last_name => @lname}) 
    end 

    def attribs 
    @person = Entry.find(:last) 
    @fname = @person.first_name 
    @lname = @person.last_name 
    @person.update_attributes({:address => params[:st_name], 
     :salary => params[:salary], :loan => params[:loan], 
     :loan_reason => params[:reason]}) 
    if !@person.address.nil? then render "show" end 
    end 

def show 
    @person = Entry.find(:last) 
end 

    def modify 
    @person = Entry.find(:last) 
    @fname = @person.first_name 
    @lname = @person.last_name 
    @entry = Entry.create({:first_name => @fname, :last_name => @lname, 
          :salary => params[:salary], :loan => params[:loan]}) 
    end 

def borrow 
    @person = Entry.find(:last) 
    if !@person.salary.nil? then 
     if (@person.salary * 3) < @person.loan 
      then @message = "You have asked for too much" 
     else @message = "No problem" 
     end 
    else @message = "empty record?" 
    end 
end 


end 

Answer 1

• 사례에 대한 before_filter를 사용하여 반복적으로 모든 변수에 인스턴스 변수를하지 마십시오 @person = Entry.find(:last)

• 를 사용하는 미리 감사드립니다 보기에서 @fname을 수행하면 @entry.first_name을 수행 할 수 있습니다.

• 인라인 if then을 사용하지 마십시오. do_something if condition을 사용하십시오. 다른 if에서도 then을 삭제하십시오.

• Event 만들기를 모델로 옮길 수도 있습니다. self.create_from_person_and_modify_params

• 사용 Event.last 대신 Event.find(:last)

• 의 사용 if @person.salary 대신 if !@person.salary.nil?

• 같은 뭔가가 모델로 if (@person.salary * 3) < @person.loan 조건을 이동합니다. asks_for_reasonable_raise?