B4cf145acfab95d0a6926b988f93b4b8

Here is a FormMailer that I have. As you know, ActionMailer is a Model not a controller. But I have yet to figure out how to reduce this to one method instead of 20...

class FormMailer < ActionMailer::Base

  def contact_us(params)
    from params[:email]
    recipients params[:to]
    subject params[:subject]
    body :params => params
  end

  def help_request(params)
    from params[:email]
    recipients params[:to]
    subject params[:subject]
    body :params => params
  end

  def service(params)
    from params[:email]
    recipients params[:to]
    subject params[:subject]
    body :params => params
  end

  def customer_care(params)
    from params[:email]
    recipients params[:to]
    subject params[:subject]
    body :params => params
  end

  def tellafriend(params)
    from params[:from_email]
    recipients params[:friend_email]
    headers 'Disposition-Notification-To' => params[:from_email] if params[:notify] == 'yes'
    subject params[:subject]
    body :params => params
  end

  # snip... 20 more
end

Refactorings

No refactoring yet !

D41d8cd98f00b204e9800998ecf8427e

Emmett

March 2, 2008, March 02, 2008 02:08, permalink

1 rating. Login to rate!

You can probably make this even more dry with alias_method, but I'm not sure how that would interact with the magic ActionMailer methods.

class FormMailer < ActionMailer::Base
  def send_email(params)
    from params[:email]
    recipients params[:to]
    subject params[:subject]
    body :params => params
  end

  def customer_care(params) send_email(params); end
end
0218fde3a78fadbadb566bdb40d7b0dd

Barry Hess

March 2, 2008, March 02, 2008 04:00, permalink

2 ratings. Login to rate!

You could try some meta-programming. This way your list of method names is nice and centralized.

class FormMailer < ActionMailer::Base

  %w{contact_us help_request service customer_care}.each do |method_name|

    define_method(method_name) do |params|
      from params[:email]
      recipients params[:to]
      subject params[:subject]
      body :params => params
    end

  end

end
991b8de70c5edb062ddfe8cd9b59277e

namxam

March 2, 2008, March 02, 2008 20:36, permalink

No rating. Login to rate!

Haven't had much sleep and therefore my mind might have produced some crappy idea, but maybe might be able to elaborate a little bit more on it. What I would suggest is using the method_missing functionality in order to create the method. This might interfere with the parents implementation for creating the "deliver_" prefixed methods. But I am not that into metaprogramming. Moreover, one might wanna add some check if the rendering of the template works.

def method_missing(m, *args)      
  unless m.to_s[0, 8] == 'deliver_'
    from params[:email]
    recipients params[:to]
    subject params[:subject]
    body :params => params
  end
end
E4b8032efbe7fd5b205187abf97abb6c

Bryce

November 13, 2009, November 13, 2009 18:37, permalink

No rating. Login to rate!

This seems like a job for filters. Unfortunately, ActionMailer isn't ActionController, and doesn't have them. But if you're feeling lucky, there is a plugin.

http://www.pathf.com/blogs/2009/01/actionmailer-callbacks-in-the-spirit-of-actioncontroller-filters/

I guess you'd end up with something like:

class FormMailer < ActionMailer::Base
  before_deliver :prep_email

  def contact_us(params)
  end

  def help_request(params)
  end

  def service(params)
  end

  def customer_care(params)
  end

  def tellafriend(params)
    headers 'Disposition-Notification-To' => params[:from_email] if params[:notify] == 'yes'
  end

  protected
  def prep_email
    from params[:from_email]
    recipients params[:friend_email]
    subject params[:subject]
    body :params => params
  end
end

Your refactoring





Format Copy from initial code

or Cancel