重构Rails代码

在以前写的博文部署应用到Heroku时的问题里有这么一段话：

股票功能需要导入交割单文件，因为导入后的文本文件不再使用，可以把上传路径由public/uploads改为tmp，这样就避免了Heroku不能写文件的问题。

下面是那时候写的导入代码：

class StocksController < ApplicationController  UPLOADS_DIRECTORY = File.join("#{Rails.root}", "public/uploads")  def import    filename = upload_file(params[:file])    lines = File.readlines("#{UPLOADS_DIRECTORY}/#{filename}")    lines.each do |line|      # do something    end    redirect_to stocks_url  end  protected    def upload_file(file)      if !file.original_filename.empty?        @filename = get_file_name(file.original_filename)        Dir.mkdir("#{UPLOADS_DIRECTORY}") unless Dir.exist?("#{UPLOADS_DIRECTORY}")        File.open("#{UPLOADS_DIRECTORY}/#{@filename}", "wb") do |f|          f.write(file.read)        end        return @filename      end    end    def get_file_name(filename)      if !filename.nil?        # does not support chinese filename?        Time.now.strftime("%Y%m%d%H%M%S") + '.txt'      end    endend

这里的实现方法是先将上传文件保存到服务器上应用的public/uploads目录中，然后再读取和处理。

其实根本不需要写的这么复杂，因为导入的文件被读取一次后就不再使用了。所以在当时写代码的时候一直有这样的想法，如果能直接获得上传文件的数据，那么就不需要再另外去写保存和读取文件的代码了。

事实也是如此。通过表单提交的file数据会首先在服务器上形成临时文件，这时其实可以通过临时文件的路径来读取上传文件的数据。

根据该想法重构后的代码如下：

class StocksController < ApplicationController  def import    lines = File.readlines(params[:file].tempfile.to_path.to_s)    lines.each do |line|      # do something    end    redirect_to stocks_url  end

重构后的代码果然清爽多了，不过还是有改进的空间。

作为控制器，controller只是负责接收request，并返回response。而具体的业务逻辑，则应该交由model去完成。下面是依照该理念再次重构后的代码：

class StocksController < ApplicationController  def import    Stock.import(params[:file])    redirect_to stocks_url  endendclass Stock < ActiveRecord::Base  def self.import(file)    lines = File.readlines(file.tempfile.to_path.to_s)    lines.each do |line|      # do something    end  endend