Last Updated: February 25, 2016
·
684
· rebyn

Prefer guard clause to nested conditions

I submitted my first Rails PR today. There is an example of guard clause usage there that I think deserves a writeup:

My first code looked like this:

def check_validity!
  if through_reflection.nil?
    raise HasManyThroughAssociationNotFoundError.new(active_record.name, self)
  end

  if through_reflection.polymorphic?
    raise HasManyThroughAssociationPolymorphicThroughError.new(active_record.name, self)
  end

  if has_one? && through_reflection.polymorphic?
    raise HasOneAssociationPolymorphicThroughError.new(active_record.name, self)
  end

  ...
  check_validity_of_inverse!
end

This gave me a failed spec. Reason was that if through_reflection.polymorphic? is true, my case of has_one? && through_reflection.polymorphic? was never to be of concern.

So I moved it up:

def check_validity!
  if through_reflection.nil?
    raise HasManyThroughAssociationNotFoundError.new(active_record.name, self)
  end

  if has_one? && through_reflection.polymorphic?
    raise HasOneAssociationPolymorphicThroughError.new(active_record.name, self)
  end

  if through_reflection.polymorphic?
    raise HasManyThroughAssociationPolymorphicThroughError.new(active_record.name, self)
  end

  ...
  check_validity_of_inverse!
end

Code worked. Specs passed. Time to do a bit of refactoring:

def check_validity!
  if through_reflection.nil?
    raise HasManyThroughAssociationNotFoundError.new(active_record.name, self)
  end

  if through_reflection.polymorphic?
    raise HasManyThroughAssociationPolymorphicThroughError.new(active_record.name, self) if has_one?
    raise HasOneAssociationPolymorphicThroughError.new(active_record.name, self)
  end

  ...
  check_validity_of_inverse!
end

Mucha better!