rubocop / rails-style-guide

A community-driven Ruby on Rails style guide
http://rails.rubystyle.guide
6.48k stars 1.06k forks source link

Use more specific predicates instead of vague `blank?` and `present?` #291

Open pirj opened 3 years ago

pirj commented 3 years ago

If puzzles me what the type of the object is when I read something like:

user = find_user
return if user.blank?

or

results = find_matches
suggest(results) if results.present?

Usually, it's [Array, nil] or [Model, nil], and usage of present?/blank? is superfluous.

Array/Hash/NilClass/TrueClass/FalseClass/String and an arbitrary Object define those methods, disguising the exact object type. blank? and present? in the Rails Guides

I'd suggest:

  1. using .nil? or an implicit check when it's only important if it's nil or not:

    return if user.nil?
    # or
    return unless user
  2. using any?/empty?/none? when distinguishing between an empty and non-empty Array/Hash

  3. for ActiveRecord::Relation, see https://github.com/rubocop/rails-style-guide/issues/232#issuecomment-425848425 3.1 using any? over present? 3.2 using empty?/none? over blank?

  4. stop using blank?/present? for booleans

String's blank?/present? are a special case, a string with whitespace only is blank? but not empty?.

cc @marcandre

pirj commented 2 years ago

@rubocop/style-guide-editors @koic WDYT?

andyw8 commented 2 years ago

The only one I really have a strong support for 4. The others can certainly result in clunky code, but I think restricting them may be overly-prescriptive.