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

• 在重复使用 @person = Entry.find(:last) 的情况下使用 before_filter

• 不要让每个变量都成为实例变量:for name : 你不需要访问 @fname在 View 中，您可以执行 @entry.first_name如果需要的话。

• 不要使用内联 if then , 使用 do_something if condition .同时删除 then从你的另一个if也是。

• 您可能想要移动 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?