Ruby重構

Question

我正在製作一個簡單的rails站點，它將存儲一些日期並執行基本的條件檢查。我在下面寫了幾個方法，並被告知我可以使它們更有效率。我一直在抓我的頭，我不知道如何做到這一點。我應該使entry.find全球？還是有更明顯的解決方案？在此先感謝

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)

• 不要讓每一個變量的實例變量：爲name：你不需要ACCES @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?