11

Railscasts put out a great episode on refactoring. One method is to move complex controller logic into service objects instead of pushing it down the model. In one service object, the following code is used:

class PasswordReset
  attr_reader :user

  def self.from_email(email)
    new User.find_by_email(email)
  end

  def self.from_token(token)
    new User.find_by_password_reset_token!(token)
  end
  ...
end

What does the new key word serve in both method bodies? new User.find_by_. How is that different from User.find_by_ ?

Here's the calling code:

  def create # controller
    password_reset = PasswordReset.from_email(params[:email])
    if password_reset.user
      password_reset.send_email
      redirect_to root_url, notice: "Email sent with password reset instructions."
    else
      redirect_to new_password_reset_url, alert: "Email address does not match a user account."
    end
  end

Also, why the attr_reader :user needed?

1
  • Can you rewrite the title so it reflects the ONE actual question you want to know about? You'll catch more fish by using bait that they find appealing. Apr 1, 2013 at 15:24

1 Answer 1

12

the classname is implicit in self methods. The code could have be written like:

 def self.from_email(email)
    PasswordReset.new User.find_by_email(email)
 end

To answer the 2nd half of your question, attr_reader defines an instance variable and a reader method (aka getter method if you're coming from java or c#). Putting it all together, you could have written it as :

class PasswordReset


  def user
    @user
  end

  def self.from_email(email)
    PasswordReset.new User.find_by_email(email)
  end

  def self.from_token(token)
    PasswordReset.new User.find_by_password_reset_token!(token)
  end
  ...
end

This is assuming PasswordReset#initialize takes a User as a parameter, and sets @user accordingly

8
  • 1
    @dae, User.find_by_email(email) is argument for PasswordReset constructor.
    – Fivell
    Apr 1, 2013 at 14:43
  • I understand now. Thank you. It seems weird, though, that you are instantiating an object than instantiates itself. Is there a name for this pattern or is it normal? Sorry, I know I'm slow. I'm not used to thinking in Ruby, I guess. I assume the attr_reader returns @user or nil? Does it just look for something called user in the instance?
    – dee
    Apr 1, 2013 at 15:25
  • 2
    In a code review I'd even argue that using the bare new is non-intuitive and forces anyone doing support to read the whole method, and maybe even the class, just to see what's going on. Using PasswordReset.new is much clearer and self-documenting. Even new(...) with the parameter in parenthesis would have been an improvement, so -1 for whoever originally wrote the code, and +1 for suggesting a cleaner/clearer solution. Apr 1, 2013 at 15:26
  • Would anyone be kind enough to answer my comment?
    – dee
    Apr 1, 2013 at 15:40
  • 1
    @theTinMan: If somebody doesn't know that self is the implicit receiver in Ruby, they probably shouldn't be doing support in the first place! Apr 1, 2013 at 18:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.