Refactoring example: single level of abstraction

# Goal: when customer first logs in, see if they recently opted out of email.
#  If yes, show a nice message encouraging them to opt back in.
#  self.current_user returns the currently logged-in user as an ActiveRecord model.

# version 1

# in CustomersController

def show
  if self.current_user.e_blacklist?  &&
      self.current_user.valid_email_address?  &&
      !(m = Option.value(:encourage_email_opt_in)).blank?
    m << ' Click the Billing Address tab (above) to update your preferences.'
    flash[:notice] ||= m
  end
end

# Problems:
#  - mixes levels of abstraction
#  - exposes implementation details of how we compute whether customer
#       needs to see this message (ie of what "opted out of email" means)
#  - how do we know what flash[:notice] was coming in?  if it was non-nil,
#       this will never do anything - yet now requires us to know this
#  - what we really want is once per login - this doesn't do that


#  version 3

# in ApplicationController

def login_message
  encourage_opt_in_message if self.current_user.has_opted_out_of_email?
end
#
# ....
#
def encourage_opt_in_message
  m = Option.value(:encourage_email_opt_in)
  m << ' Click the Billing Address tab (above) to update your preferences.' unless m.blank?
  return m
end


# in customer.rb

def has_opted_out_of_email?
  e_blacklist? && valid_email_address?
end

# in handler for Login action

flash[:notice] = login_message || "Logged in successfully"