CODEWITHSUNDEEP
ruby-on-railsactiverecord
samol
samol

Reputation: 20640

Elegant alternative to after_create callbacks in Rails?

I have two models

class Contract < ActiveRecord::Base
  has_many :transactions
end

class Transaction < ActiveRecord::Base
  belongs_to :contract
  after_create :mark_contract_as_live
  def mark_contract_as_live
    k = self.contract
    if !k.is_live
      k.update_attributes(:is_live => true)
    end
  end
end

is_live is a boolean field in the Contract model. A contract is defaulted to not live (is_live => false) when it created. When the first transaction is recorded it is marked as live (is_live => true). With the solution, I have above, it means that every transaction creation requires calling the database to check whether contract is live. Is there an alternative to this?

If contract has thousands of transactions, that means this will be called thousands of times although it is only relevant to the very first transaction.

In a general sense, what is an elegant way to implement callbacks. This seems messy?

Upvotes: 4

Views: 1622

Answers (1)

Damien
Damien

Reputation: 27493

class Contract < ActiveRecord::Base
  has_many :transactions

  def mark_as_live
    update(is_live: true) unless is_live?
  end
end

class Transaction < ActiveRecord::Base
  belongs_to :contract

  after_create :mark_contract_as_live

private

  def mark_contract_as_live
    contract.mark_as_live
  end
end

It is the Contract class responsibility to care if a contract should be marked as live or not. The Transaction class should not handle this. So I created a mark_as_live in the Contract class and call it in the Transaction after_create callback.

I would prefer to use a guard clause in the mark_as_live method like so:

def mark_as_live
  return if is_live?

  update(is_live: true)
end

But because it is a very short method, it probably does not worth it.

Note also that ActiveRecord adds methods like xxx? for boolean field. A question mark at the end of the method conveys more clearly what you want to say.

Finally, but this is a question of taste, I dislike prefixing my boolean attributes with is_xxx. I don't use RSpec and may be wrong but I think that it adds some predicate matchers like be_xxx for the xxx attribute and it may go weird with the is_xxx attributes. Because a lot of people are using RSpec, it may become a bit of a convention.

If contract has thousands of transactions, that means this will be called thousands of times although it is only relevant to the very first transaction.

The Contract instance will be still loaded if you create a transaction like so: contract.transactions.create(transaction_params). So the call to is_live? will come at no cost, you don't have to worry.

Upvotes: 5

Related Questions