CODEWITHSUNDEEP
ruby-on-railsif-statement
user1128912
user1128912

Reputation: 191

Ruby if condition - simplify

newbie here.

I'm trying to simplify this and I'm wondering If a ternary statement should be the way to go. Also, I guess I should not repeat "PaymentAccount.new". As I try to convert it into a ternary statement I keep getting errors.

def initialize(document_data)
  document_data.payment_accounts =
    if document_data.compensation_currency == 'EUR'
      [
        PaymentAccount.new(
          currency: 'EUR',
          bank_name: 'bank name',
          iban: 'EU000000000000001',
          swift: 'EURBANK'
        )
      ]
    else
      [
        PaymentAccount.new(
        bank_name: 'bank name 2',
        iban: 'NT00000000000000',
        swift: 'NTBANK'
        )
      ]
    end

  super(document_data)
end

Upvotes: 0

Views: 137

Answers (1)

obiruby
obiruby

Reputation: 419

IMO it's a little smelly to implement in that way. A couple things immediately stick out:

  • Modifying input params is (usually) bad practice and a sign to refactor
  • Fat initializers may make code maintainability more difficult over time
  • Are you sure you want to reset the value of document_data.payment_accounts every time the parent class is initialized?

Assuming document_data and payment_accounts are both references to Active Record objects, I would consider something like the following:

class PaymentAccount
  BANK_NAME_1 = {currency: 'EUR',
          bank_name: 'bank name',
          iban: 'EU000000000000001',
          swift: 'EURBANK'}
    
  BANK_NAME_2 = { bank_name: 'bank name 2',
          iban: 'NT00000000000000',
          swift: 'NTBANK'}

  def self.bank_params(data)
    data.compensation_currency == "EUR" ? BANK_NAME_1 : BANK_NAME_2
  end
end

class DocumentData # or whatever model `document_data` belongs to
  after_initialize :ensure_payment_accounts

  def ensure_payment_accounts
    payment_accounts.find_or_initialize_by PaymentAccount.bank_params(self)
  end
end

I feel an approach like this may be simpler, since we're not overriding any initializers here. We are using an after initialize callback, but based off your question, perhaps you need it that way.

Note too that in an ideal world the bank param info would live in a separate config file. That way, if you ever need to change those params, you don't even need to bother with the business logic of your implementation.

Upvotes: 2

Related Questions