Last Updated: February 25, 2016
·
739
· 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!