What is the cleanest way of writing a function with conditional statement with many returning options?

Question

Maybe that's simple, but I'm a little confused.

I have such a Ruby code:

def my_function(found_objects)
  # ...
  if found_objects.second
    return CoreObjectFactory.get_object(found_objects.second, found_objects.first)
  else
    return CoreObjectFactory.get_object(found_objects.first)
  end
  # ...
end

I left return keywords for clarity, but there aren't the biggest problem.

According to Uncle Bob (edit: I'm sorry, that was not his opinion, I messed something) every function should have only one ending, so I'm trying to refactor my code. The simplest solution is this:

def my_function(found_objects)
  # ...
  if found_objects.second
    result = CoreObjectFactory.get_object(found_objects.second, found_objects.first)
  else
    result = CoreObjectFactory.get_object(found_objects.first)
  end
  # ...
  result
end

But here Rubocop tells me:

Use the return of the conditional for variable assignment and comparison. (convention:Style/ConditionalAssignment)

It seems to suggest something like this:

def my_function(found_objects)
  # ...
  result = if found_objects.second
             CoreObjectFactory.get_object(found_objects.second, found_objects.first)
           else
             CoreObjectFactory.get_object(found_objects.first)
           end
  # ...
  result
end

Although in my opinion it's really creepy. Number of spaces intending this code is odd, what makes it looking bad and harder to work with Tab (nested intendations inteds only one space by default).

But I could write the same this way:

def my_function(found_objects)
  # ...
  result = if found_objects.second
    CoreObjectFactory.get_object(found_objects.second, found_objects.first)
  else
    CoreObjectFactory.get_object(found_objects.first)
  end
  # ...
  result
end

It looks better for me, but unfortunately not for Rubocop, which has three conventions against me. Of course I can ignore it... but it still seems not perfect.

Layout/IndentationWidth:
  Enabled: false
Layout/ElseAlignment:
  Enabled: false
Layout/EndAlignment:
  Enabled: false

I could also use a ternary operator, but Rubocop doesn't like long lines and multiline ternary opeators.

if found_objects
  # ...
  result = found_objects.second ?
    CoreObjectFactory.get_object(found_objects.second, found_objects.first) :
    CoreObjectFactory.get_object(found_objects.first)
  # ...
  result
end

So, is there some way to avoid all problems above?

Answer 1

According to Uncle Bob every function should have only one ending

This is a weird corruption of an old rule from Dijkstra that says that functions should only have a single entry and a single exit. What this really refers to is in old Fortran and assembly language programming where you could enter the function at multiple places and when you leave the function, return to different places (ie. not the original call site). Note that this is not the same as returning to the same place from multiple different exit points within the function.

You should feel fine writing multiple returns if it's clearest to you and your team. Multiline ternary operators can be an eyesore and there doesn't appear to be a way to move the ternary inside the function call.

Answer 2

First code is meant to be read. If the code is hard for you to read, then even if millions of other people are quite happy reading the code, something is still wrong. Whatever you do choose to go with, it should be ledgeable to you first.

Second, RuboCop and all of the other style systems are formulaic. A lot of good code uses those same layouts and conventions that have been distilled into those formulas. But so does a lot of bad code. Just following those formulas turnkey will not necessarily make good code, just help you avoid some well known sources of bad code.

Third, Good code is prose. It is words and symbols that explain themselves as much by their content as by their layout. You yourself can see that in your own analysis. You intuitively know that the first and second examples communicate with you better than the if expression, or the ternary operator.

To avoid the problem above, stop adhering to styling advice blindly. Particularly when it reduces the simplicity or clarity of the code. This also means that RuboCop must be demoted to an advisory tool, it should not ever be used as a quality gate (in so far as its styling capacity). It is simply an aid to highlight code that could use some communication polish.