Duplication Removal

_description.md

Which version of the code do you find more clear and/or preferable?

balance.rb

module Ledger::Balance

  def balance(transaction=nil)
    sum_of_credits(transaction) - sum_of_debits(transaction)
  end

  def amount_available(transaction=nil)
    self.balance(transaction)
  end

  def spent
    sum_of_debits
  end

protected

  def sum_of_credits(transaction=nil)
    Ledger::Credit.where(destination: self).
      where(transaction ? ['transaction_id <= ?', transaction] : {}).
      select('COALESCE(SUM(credit),0) as credit')[0].credit
  end

  def sum_of_debits(transaction=nil)
    Ledger::Debit.where(source: self).
      where(transaction ? ['transaction_id <= ?', transaction] : {}).
      select('COALESCE(SUM(debit),0) as debit')[0].debit
  end
end

balance_duplication_removed.rb

module Ledger::Balance

  def balance(transaction=nil)
    sum_of_credits(transaction) - sum_of_debits(transaction)
  end

  def amount_available(transaction=nil)
    self.balance(transaction)
  end

  def spent
    sum_of_debits
  end

protected

  def sum_of_credits(transaction)
    sum_of(:credit, transaction)
  end

  def sum_of_debits(transaction)
    sum_of(:debit, transaction)
  end

  def sum_of(type, transaction)
    owner = SOURCE_DESTINATION[type]
    model = "Ledger::#{type.to_s.camelize}".constantize

    model.where("#{owner} = ?", self.id).
      where(transaction ? ['transaction_id <= ?', transaction] : {}).
      select("COALESCE(SUM(#{type}),0) as #{type}")[0].send(type)
  end

  SOURCE_DESTINATION = {
    credit: 'destination_id',
    debit: 'source_id'
  }
end

@brianhogg I actually prefer to read it in this order. I see that sum_of_credits calls sum_of, and sum_of follows shortly after. Otherwise I don't have context for what sum_of is for.

@adamwathan, I also prefer the first I think. In this case, loading into memory and summing there isn't really an option. Interesting thoughts about how they are calculated though. I can't foresee them changing separately but I guess it is possible. I agree with you about if there were 3 the second option would be nicer. While doing this I had in mind a quote from Sandi Metz that "duplication is cheaper than the wrong abstraction", and I feel like it's pretty applicable here.

But I want my perfect 4.0 GPA on Code Climate! :)