Ruby if condition - simplify

Question

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

Ternary is usually only used when you have one liners or very simple conditions, not for whole blocks of code as you have there. — Eyeslandic
– Eyeslandic, Commented Nov 21, 2021 at 9:33
Most probably the first thing to do would be to move the big bunch of code in the constructor (initialize) to somewhere else in the class itself. It might work better for you if you don't add complicated business logic like this one to your constructors. — Sebastián Palma
– Sebastián Palma, Commented Nov 21, 2021 at 10:43

obiruby · Accepted Answer · 2021-11-21 14:19:29Z

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.

Collectives™ on Stack Overflow

Ruby if condition - simplify

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related