CODEWITHSUNDEEP
ruby-on-railsrubyruby-on-rails-4methodscallback
gosseti
gosseti

Reputation: 975

Rails refactor/combine similar before_update callback methods into one

I’ve created some model callback methods that timestamp an attribute every time a file is created, changed or removed.

I was wondering whether there was a more elegant way to write these callbacks, without having to repeat similar method code five times:

before_update :quote_file_updated?, on: [:create, :update], if: ->(r) { r.quote_file_changed? }
before_update :survey_file_updated?, on: [:create, :update], if: ->(r) { r.survey_file_changed? }
before_update :sign_off_sheet_file_updated?, on: [:create, :update], if: ->(r) { r.sign_off_sheet_file_changed? }
before_update :invoice_file_updated?, on: [:create, :update], if: ->(r) { r.invoice_file_changed? }
before_update :cert_file_updated?, on: [:create, :update], if: ->(r) { r.cert_file_changed? }


def quote_file_updated?
  self.remove_quote_file ? self.quote_file_date = nil : self.quote_file_date = Time.now
end

def survey_file_updated?
  self.remove_survey_file ? self.survey_file_date = nil : self.survey_file_date = Time.now
end

def sign_off_sheet_file_updated?
  self.remove_sign_off_sheet_file ? self.sign_off_sheet_file_date = nil : self.sign_off_sheet_file_date = Time.now
end

def invoice_file_updated?
  self.remove_invoice_file ? self.invoice_file_date = nil : self.invoice_file_date = Time.now
end

def cert_file_updated?
  self.remove_cert_file ? self.cert_file_date = nil : self.cert_file_date = Time.now
end

Upvotes: 2

Views: 197

Answers (2)

gosseti
gosseti

Reputation: 975

It’s better to keep the code verbose in this case, as it’s hard to see what’s going on. The callbacks should be changed so that the before_update becomes before_save (so it’s fired on the create action too).

The callback should be:

before_save :quote_file_updated?, if: ->(r) { r.quote_file_changed? or r.remove_quote_file.to_b }

The to_b is a method that converts the 0 or 1 from the remove_quote_file to a boolean. Based on this we can set the datestamp to nil when the file is removed.

The method itself should then become:

def quote_file_updated?
  remove_quote_file.to_b ? self.quote_file_date = nil : self.quote_file_date = Time.now
end

We do a similar thing again evaluating the remove_quote_file as a boolean and then updating or removing the timestamp based on this.

Upvotes: 1

Max Williams
Max Williams

Reputation: 32933

before_update :set_updated_fields, on: [:create, :update]

def set_updated_fields
  ["quote", "survey", "sign_off_sheet", "invoice", "cert"].each do |filetype|
    if self.send("#{filetype}_file_changed?")
      self.attributes = {"#{filetype}_file_date" => self.send("remove_#{filetype}_file") ? nil : Time.now}
    end
  end
end

This is a bit incomprehensible but i think it will work.

Upvotes: 2

Related Questions