return early vs if in ruby code

Question

I see two styles of writing the same thing:

def find_nest(animal)
  return unless animal.bird?
  GPS.find_nest(animal.do_crazy_stuff)
end

vs

def find_nest(animal)
  if animal.bird?
     GPS.find_nest(animal.do_crazy_stuff)
  end
end

Which one is more correct/preferable/following-best-practises? Or it does not matter?

Toot

I'd write GPS.find_nest(animal.do_crazy_stuff) if animal.bird? – Chiu

@Chiu I always come up with terrible examples for questions :P – Toot

@Stefan: I find modifier conditionals confusing in case of expressions where value matters. Only use them on "statements" and only if the primary expression is not too long (or else it'd push the modifier out of sight) – Ineffaceable

@FilipBartuzi since your question is tagged oop – of course it would be preferable to have a Bird class (descendant of Animal) and implement Bird#find_nest without arguments as def find_nest; GPS.find_nest(do_crazy_stuff); end. – Chiu

Answer 1

As per Ruby style guide,

Prefer a guard clause when you can assert invalid data. A guard clause is a conditional statement at the top of a function that bails out as soon as it can.

# bad
def compute_thing(thing)
  if thing[:foo]
    update_with_bar(thing)
    if thing[:foo][:bar]
      partial_compute(thing)
    else
      re_compute(thing)
    end
  end
end

# good
def compute_thing(thing)
  return unless thing[:foo]
  update_with_bar(thing[:foo])
  return re_compute(thing) unless thing[:foo][:bar]
  partial_compute(thing)
end

Rf

Answer 2

It is obviously a matter of personal preference. But I prefer the early return. Not only does it make code "flatter" and easier to read, it also scales well with the number of checks. For example:

  def create_terms_of_service_notification
    return if Rails.env.test?
    return if current_user.accepted_tos?
    # imagine 5 more checks here. 
    # Now imagine them as a mess of nested ifs.

    # create the notification
  end

Ineffaceable

Answer 3

This :}

def find_nest(animal)
  GPS.find_nest(animal.do_crazy_stuff) if animal.bird?
end

Filefish