makandra / makandra-rubocop

makandra's default Rubocop configuration
MIT License
6 stars 1 forks source link

Allow methods prefixed with has_ #6

Closed codener closed 5 years ago

codener commented 5 years ago

Rubocop won't allow a method named has_replies?:

class Message
  def reply?
    # return whether message IS a reply
  end

  def has_replies?
    # return whether message HAS any replies
  end
  ...
end

# => app/models/message.rb:28:7: C: Naming/PredicateName: Rename has_replies? to replies?

However, changing this would make the model quite hard to understand. #reply? and #replies? sound so similar, yet have quite a different meaning. Furthermore, replies could also be meant as 3rd person ("it replies"), not plural.

Verdict

I believe we should not force both property ("is") and possession ("has") into unprefixed query methods. While an "is" prefix is not needed in Ruby, the "has" prefix definitely has its place.

foobear commented 5 years ago

FYI: The cop steps from the idea of avoiding redundant and inconsistent method name prefixes which don't match core Ruby naming (e.g. has_key? became key? a while ago).

I personally don't consider similarity issues a valid point here and would be perfectly fine with replies? as a means of expressing "this message has replies", or even prefer any_replies? because it more clearly expresses what you actually mean (IMO).

However, I agree that there are are reasons for has_-prefixed method names and suggest we adjust the method's NamePrefixBlacklist accordingly. An is_ prefix should still be a violation.

foobear commented 5 years ago

Cop has been disabled in 3.1.0