5
\$\begingroup\$

How should I make this more Ruby-like or just "better"?

def password_format_is_valid?(submitted_password)

  #Gets regular expression for password format validation from settings and applies it
  regex = Regexp.new(Setting['global_admin.password_format_regex'])
  if submitted_password =~ regex then
    return true
  else
    self.errors.add(:password, Setting['global_admin.password_format_error_message'])
    return false
  end

end
\$\endgroup\$
4
\$\begingroup\$

It seems like you need a Rails validation callback (and a virtual attribute for submitted_password). I'd write:

attr_accessor :submitted_password
validate :password_format_is_valid?

def password_format_is_valid?
  regex = Regexp.new(Setting['global_admin.password_format_regex'])
  unless submitted_password =~ regex
    self.errors.add(:password, Setting['global_admin.password_format_error_message'])
    false
  end
end

Comments:

  • In the vein of Lisp, the last expression of a body in Ruby is the return value of the method/block, so no need to use an explicit return (in fact it's unidiomatic and discouraged)

  • Note that Rails can validate fields with regular expression, you should use the predefined validations whenever possible: validates_format_of.

\$\endgroup\$
  • \$\begingroup\$ Thank you for the explanation and example - much appreciated! \$\endgroup\$ – Douglas Birch Oct 10 '12 at 15:03
  • \$\begingroup\$ This returns nil when the password is valid. \$\endgroup\$ – steenslag Nov 10 '12 at 21:09
  • \$\begingroup\$ @steenslag: Indeed, I tried to write the method as a Rails callback, but the code was incomplete. Edited. \$\endgroup\$ – tokland Nov 10 '12 at 21:42
0
\$\begingroup\$

There is no need to write explicit return . You can omit it. Because in Ruby the result of last executed statement in your code is returned automatically.

\$\endgroup\$
  • \$\begingroup\$ Care to expand your answer a bit to explain why return can be omitted? \$\endgroup\$ – yannis Oct 10 '12 at 4:30

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.