Method name to explain context

Josh Forbes — Wed, 08 Jan 2020 20:04:38 +0000

We have a graphql mutation to send a user a reset password email. We return a generic success result if the user supplies an unknown email address. This is to prevent a malicious user from gaining knowledge about which emails are registered to our app:

def resolve
  if user.nil?
    {success: true}
  else
    result = CreateResetPasswordToken.new(user).call
    {success: result.success?}
  end
end

private

def user
  @user ||= User.find_by(email: input.email)
end

I think this is a pretty common approach. But the actual contents of the method could be confusing if you don't understand the context. I could picture a new developer(or myself in 6 months) coming along and thinking: "That can't be right. We are returning success when the user is not found. I bet it's a typo."

So that got me thinking about how to communicate the context in the code. One possibility is to write a test case that describes that it should be true when the user is not found. That at least provides a clue that it isn't a mistake. But then I wondered what it would look like if we used a method name to communicate what is happening.

def resolve
  if user.nil?
    success_to_conceal_which_email_addresses_exist
  else
    result = CreateResetPasswordToken.new(user).call
    {success: result.success?}
  end
end

private

def success_to_conceal_which_email_addresses_exist
  {success: true}
end

What do you think?

DEV Community: Josh Forbes

Method name to explain context