99b003c2a01aa0dc3b0fe7c5d72f56a0

I have to call the setup_email(user) method before every other method, how to reduce this kind of code duplication.

[ruby_on_rails]

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
class UserMailer < ActionMailer::Base 
  def signup_notification(user)
    setup_email(user)
    ...
  end
  
  def activation(user)
    setup_email(user)
    ...
  end
  
  def forgot_password(user)
    setup_email(user)
    ...
  end
  
  def reset_password(user)
    setup_email(user)
    ...
  end


  protected
  
  def setup_email(user)
    @recipients  = "#{user.email}"
    @from        = "#{DO_NOT_REPLY_EMAIL}"
    @sent_on     = Time.now
  end  
end

Refactorings

No refactoring yet !

5170ca260dbd2cdfd5a887a4dba7636f

Jeremy Weiskotten

April 21, 2008, April 21, 2008 14:55, permalink

No rating. Login to rate!

There's a similar set of refactorings to DRY up ActionMailers: http://refactormycode.com/codes/246-action-mailer-it-has-to-be-easier

F04aeb28129f653b207e8b5d92706096

Adkron

April 22, 2008, April 22, 2008 02:24, permalink

No rating. Login to rate!

Not sure if this is much better but I thought it was an interesting idea. Then you would make a call like prepare_mail(user, signup_notification). Good luck.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
class UserMailer < ActionMailer::Base
  def signup_notification 
    Proc.new  do  |user|
        ...
      end
  end

  def prepare_mail(user, proc)
    setup_email(user)
    proc.call(user)
  end

  protected
  
  def setup_email(user)
    @recipients  = "#{user.email}"
    @from        = "#{DO_NOT_REPLY_EMAIL}"
    @sent_on     = Time.now
  end  
end

Your refactoring





Format Copy from initial code

or Cancel