I'm adding a validation method to my Ruby on Rails model which checks the value of one field based on the value of another.

The rule is that if value_type == 'range', then a_max and b_max must be set.

My method looks like this:

def ensure_correct_type
  if value_type == 'range'
    if a_max.present? == false
      errors.add(:a_max, 'must be set for a range.'
    end
    if b_max.present? == false
      errors.add(:b_max, 'must be set for a range.'
    end
  end
end

The code works fine, however I'm getting a rubocop warning:

Style/GuardClause: Use a guard clause (return unless value_type == 'range') instead of wrapping the code inside a conditional expression.

What is the correct convention for writing something like this in Ruby? The suggestion in Robocop is to do return unless value_type == 'range', but that seems strange since the method doesn't return at all.

I did try to implement the suggestion:

def ensure_correct_type
  return unless value_type == 'range'
  
  if a_max.present? == false
    errors.add(:a_max, 'must be set for a range.'
  end
  if b_max.present? == false
    errors.add(:b_max, 'must be set for a range.'
  end
end

But then I get the same warnings for if a_max.present? == false lines... I don't think I can do return unless on that line since I want to add errors for both fields if necessary.

What's the best practice convention in Ruby for handling something like this?

CodePudding user response：

I would try:

def ensure_correct_type
  return if value_type != 'range'

  errors.add(:a_max, 'must be set for a range.') if a_max.blank?
  errors.add(:b_max, 'must be set for a range.') if b_max.blank?
end

Or you could use build-in validations like this:

validates :a_max, presence: true, if: :range_value_type?
validates :b_max, presence: true, if: :range_value_type?

private
def range_value_type?
  value_type == 'range'
end