-
Notifications
You must be signed in to change notification settings - Fork 609
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor AnnotateModels::Helpers #723
Refactor AnnotateModels::Helpers #723
Conversation
return false unless val =~ Constants::TRUE_RE | ||
|
||
true | ||
val.present? && Constants::TRUE_RE.match?(val) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def true?(val)
return false if val.blank?
return false unless val =~ Constants::TRUE_RE
true
can be refactored to
def true?(val)
if val.blank?
false
else
if val =~ Constants::TRUE_RE
true
else
false
end
end
end
So we can easily replace them to a one liner method.
I used Regexp#match?
instead of String#=~
because String#=~
returns nil
or an integer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good keep it up :)
return false unless val =~ Constants::TRUE_RE | ||
|
||
true | ||
val.present? && Constants::TRUE_RE.match?(val) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 nice
I refactored two methods in AnnotateModels::Helpers for readability.
I refactored two methods in AnnotateModels::Helpers for readability.