current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

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
share|improve this question

2 Answers 2

up vote 3 down vote

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.

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

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.

share|improve this answer
    
Care to expand your answer a bit to explain why return can be omitted? –  Yannis Oct 10 '12 at 4:30

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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