5

CSV ファイルを複数のテーブルにインポートするインポート機能を作成しています。CsvParserCSV ファイルを解析してレコードを作成するというモジュールを作成しました。作成アクションを受け取る私のモデルは、CsvParser. これらは を呼び出してCsvParser.create、正しい属性順序とオプションのラムダを渡しますvalue_parser。このラムダは、ハッシュ内の値を優先形式に変換します。

class Mutation < ActiveRecord::Base
  extend CsvParser

  def self.import_csv(csv_file)
    attribute_order = %w[reg_nr receipt_date reference_number book_date is_credit sum balance description]

    value_parser = lambda do |h|
      h["is_credit"] = ((h["is_credit"] == 'B') if h["is_credit"].present?)
      h["sum"] = -1 * h["sum"].to_f unless h["is_credit"]
      return [h]
    end

    CsvParser.create(csv_file, self, attribute_order, value_parser)    
  end
end

メソッド内のチェックの代わりにラムダを使用している理由CsvParser.createは、ラムダがこのモデルに属するビジネス ルールのようなものだからです。

私の質問は、このラムダをテストする方法です。モデルまたは CsvParser でテストする必要がありますか? self.importラムダ自体またはメソッドの配列の結果をテストする必要がありますか? 多分私は別のコード構造を作るべきですか？

私の CsvParser は次のようになります。

require "csv"

module CsvParser

  def self.create(csv_file, klass, attribute_order, value_parser = nil)
    parsed_csv = CSV.parse(csv_file, col_sep: "|")

    records = []    

    ActiveRecord::Base.transaction do
      parsed_csv.each do |row|
        record = Hash.new {|h, k| h[k] = []}      
        row.each_with_index do |value, index|
          record[attribute_order[index]] = value
        end 
        if value_parser.blank?
          records << klass.create(record)
        else
          value_parser.call(record).each do |parsed_record|
            records << klass.create(parsed_record)
          end
        end
      end 
    end
    return records
  end

end

モジュール自体をテストしています: require 'spec_helper'

describe CsvParser do

  it "should create relations" do
    file = File.new(Rails.root.join('spec/fixtures/files/importrelaties.txt'))
    Relation.should_receive(:create).at_least(:once)
    Relation.import_csv(file).should be_kind_of Array 
  end

  it "should create mutations" do
    file = File.new(Rails.root.join('spec/fixtures/files/importmutaties.txt'))
    Mutation.should_receive(:create).at_least(:once)    
    Mutation.import_csv(file).should be_kind_of Array
  end

  it "should create strategies" do
    file = File.new(Rails.root.join('spec/fixtures/files/importplan.txt'))
    Strategy.should_receive(:create).at_least(:once)
    Strategy.import_csv(file).should be_kind_of Array
  end

  it "should create reservations" do
    file = File.new(Rails.root.join('spec/fixtures/files/importreservering.txt'))
    Reservation.should_receive(:create).at_least(:once)
    Reservation.import_csv(file).should be_kind_of Array
  end

end
4

1 に答える 1

4

いくつかの興味深い質問。いくつかのメモ:

  1. おそらく、ラムダ内で戻り値を持つべきではありません。最後のステートメント [h] を作成するだけです。

  2. コードを正しく理解していれば、ラムダの 1 行目と 2 行目が複雑すぎます。それらを減らして、読みやすく、リファクタリングしやすくします。

    h["is_credit"] = (h['is_credit'] == 'B') # I *think* that will do the same
h['sum'] = h['sum'].to_f   # Your original code would have left this a string
h['sum'] *= -1 unless h['is_credit']

  3. あなたのラムダは（を除いて）外部のものに依存していないように見えるhので、個別にテストします。定数にすることもできます：

    class Mutation < ActiveRecord::Base
  extend CsvParser    # <== See point 5 below

  PARSE_CREDIT_AND_SUM = lambda do |h|
    h["is_credit"] = (h['is_credit'] == 'B') 
    h['sum'] = h['sum'].to_f
    h['sum'] *= -1 unless h['is_credit']
    [h]
  end

  4. 理論的根拠を知らなければ、このコードをどこに配置すべきかを判断するのは困難です。私の直感では、それは CSV パーサーの仕事ではないということです (ただし、優れたパーサーは浮動小数点数を検出して文字列から変換する場合があります)。CSV パーサーを再利用できるようにしておいてください。(注: 再読すると、この質問に自分で答えたと思います。それはビジネス ロジックであり、モデルに関連付けられています。直感に従ってください!)

  5. 最後に、メソッドを定義していますCsvParser.create。アクセスするために CsvParser を拡張する必要はありませんが、CsvParser に他の機能がある場合は、CsvParser.createcreate_from_csv_file のような通常のモジュール メソッドを作成することを検討してください。

于 2012-04-11T14:11:15.767 に答える