5

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?

sawa
  • 165,429
  • 45
  • 277
  • 381
Filip Bartuzi
  • 5,711
  • 7
  • 54
  • 102
  • I'd write `GPS.find_nest(animal.do_crazy_stuff) if animal.bird?` – Stefan Jun 17 '16 at 11:45
  • @Stefan I always come up with terrible examples for questions :P – Filip Bartuzi Jun 17 '16 at 11:47
  • @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) – Sergio Tulentsev Jun 17 '16 at 11:49
  • 1
    @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`. – Stefan Jun 17 '16 at 11:59

3 Answers3

27

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
wovano
  • 4,543
  • 5
  • 22
  • 49
Wand Maker
  • 18,476
  • 8
  • 53
  • 87
6

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
Sergio Tulentsev
  • 226,338
  • 43
  • 373
  • 367
1

This :}

def find_nest(animal)
  GPS.find_nest(animal.do_crazy_stuff) if animal.bird?
end
ringarifullin
  • 15
  • 4