0
\$\begingroup\$

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

\$\endgroup\$
3
\$\begingroup\$

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
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.