rails method for transaction type

Question

I am trying to make a rake task but after completion my code i realized i need some refactorization. How could I do refactorization of the following code?

def self.transaction_type(row)
    if row['SENDER_BLZ'] == '00000000' and row['RECEIVER_BLZ'] == '00000000'
      return 'AccountTransfer'
    elsif row['SENDER_BLZ'] == '00000000' and row['UMSATZ_KEY'] == '10'
      return 'BankTransfer'
    elsif row['RECEIVER_BLZ'] == '70022200' and ['16'].include?row['UMSATZ_KEY']
      return 'Lastschrift'
    else
      return false
    end
  end

and second method is

def self.import_file_row_with_error_handling(row, validation_only, errors, dtaus)
  error_text = nil
  self.import_retry_count = 0
  5.times do
    self.import_retry_count += 1
    error_text = nil
    begin
      import_file_row(row, validation_only, errors, dtaus)
      break
    rescue => e
      error_text = "#{row['ACTIVITY_ID']}: #{e.to_s}"
      break
    end
  end
  errors << error_text if error_text

  [errors, dtaus]
end

Just need bit guid. thanks

Answer 1

It looks like row is simply a hash, but it has various value combinations that are significant.

You could refactor the detection of those combinations, for example by creating a class based on the row that has an attribute for #transaction_type ...

class Row
  def initialize(row)
    @row = row
  end

  def sender_blz
    row['SENDER_BLZ']
  end

  def receiver_blz
    row['RECEIVER_BLZ']
  end

  def umsatz_key
    row['UMSATZ_KEY']
  end

  def transaction_type
    return "AccountTransfer" if sender_blz   == "00000000" && receiver_blz == "00000000"
    return "BankTransfer"    if sender_blz   == "00000000" && umsatz_key == "10"
    return "Lastschrift"     if receiver_blz == "70022200" && umsatz_key == "16"
    false
  end
end

Then your method becomes

Row.new(row).transaction_type

I would rather see magic strings like "00000000" replaced with constants, and conditions like sender_blz == "00000000" replaced with methods that give a name to the condition, like:

def sender_not_specified?
  sender_blz == NOT_SPECIFIED
end

... thus you would have something like ...

  def transaction_type
    return "AccountTransfer" if sender_not_specified? && receiver_not_specified?
    return "BankTransfer"    if sender_not_specified? && something_about_umsatz?
    return "Lastschrift"     if receiver_not_specified? && something_else_about_umsatz?
    false
  end