4
\$\begingroup\$

I have this authenticate! method where I am trying to retrieve their account based on the subdomain, then find the user. if user is found & their password is matched, we return success, else fail in all other cases. here is the code:

def authenticate!
    account = Account.find_by(subdomain: subdomain)
    if account
      u = account.users.find_by email: params["user"]["email"]
      if u.nil?
        fail!
      else
        u.authenticate(params["user"]["password"]) ? success!(u) : fail!
      end
    else
      fail!
    end
end

Now, I could simply this further by moving subdomain into a helper method,

def subdomain
  ActionDispatch::Http::URL.extract_subdomains(request.host, 1)
end

def authenticate!
      account = Account.find_by(subdomain: subdomain)
      if account
        u = account.users.find_by email: params["user"]["email"]
        if u.nil?
          fail!
        else
          u.authenticate(params["user"]["password"]) ? success!(u) : fail!
        end
      else
        fail!
      end
end

But how to I simply these nested conditionals?

\$\endgroup\$
8
\$\begingroup\$

Another view on @tokland solution:

def authenticate!
  return fail! unless account = Account.find_by(subdomain: subdomain)
  return fail! unless user = account.users.find_by(email: params["user"]["email"])
  return fail! unless user.authenticate(params["user"]["password"])
  success! user
end

This intention is to be as close to "original task explained in common English" as possible. And return keyword allows me to do that.

\$\endgroup\$
  • 1
    \$\begingroup\$ While this is probably the more idiomatic in Ruby (in the sense that is what most Ruby programmers would write), and saves some lines, personally I dislike (enormously) 1) early returns, 2) inline conditionals, for a piece of code that is an expression. My rationale: code.google.com/p/tokland/wiki/RubyFunctionalProgramming \$\endgroup\$ – tokland Sep 23 '13 at 18:51
  • \$\begingroup\$ I don't care, what do most Ruby programmers write. My way is to be as close to "original task explained in common English" as possible. And return keyword allows me to do that. \$\endgroup\$ – Nakilon Sep 24 '13 at 13:18
3
\$\begingroup\$

Some programmers don't like the pattern if (var = value) (for understandable reasons), unfortunately, by avoiding it at all costs, you end up writing verbose code like yours. If you have no problems with it:

def authenticate!
  if (account = Account.find_by(subdomain: subdomain)) &&
     (user = account.users.find_by(email: params["user"]["email"])) &&
     user.authenticate(params["user"]["password"])
    success!(user)
  else
    fail!
  end
end

Note that the parens are there to make more clear that we are doing an assignment and not a comparison (and to avoid operator precendence problems)

\$\endgroup\$
1
\$\begingroup\$

Symantic variant of @Nakilon's answer:

  def authenticate!
    user = nil
    return success!(user) if
      account = Account.find_by(subdomain: subdomain)) and
      user = account.users.find_by(email: params["user"]["email"]) and
      user.authenticate(params["user"]["password"])
    return fail!  
  end

I prefer @Nakilon, but thought this might be of some interest. Note: 1) user must be initialized (to anything); and 2) the line continuation mark \ is not needed where it's obvious to Ruby that the statement is not complete. I don't know if the latter conforms to the Ruby style guide.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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