Ruby one line if return statement

Question

is there a way to shorten this line on Ruby?

if (res = bla_permission_invalid).is_a? String then return res end

on

def something # many things that like this
  if (res = bla_permission_invalid).is_a? String then return res end
  # do something else
  return true
end

when the content of bla_permission_invalid are something like

def bla_permission_invalid
  return invalid_address_report_func if invalid_address?
  return permission_error_report_func if @user.not_one_of? [ :group1, :group2 ]
  return nil
end

invalid_adress_report_func and permission_error_report_func returns string

Solution 1

If possible values are String and NilClass, then the code can be simplified to this:

def something
  res = bla_permission_invalid()
  return res if res # strings are truthy, so they'll be returned but nil will proceed

  # do something else
  true
end

Solution 2

def something
  bla_permission_invalid || (
  # do something else
  true)
end

Solution 3

For fun, one could rewrite your something method like this:

def something
  true.tap do
    bla_permission_invalid.tap { |res| return res if res.is_a? String }
    # do something else (thx Sergio)
  end
end

But more importantly, Mark Thomas deserves credit for his observation, that the problem at hand should be solved by using custom exceptions.

Error code approach is good in languages that don't have exceptions. Ruby has them.

Solution 4

Mark Thomas already noted in his comment that it looks like you're trying to handle errors on your own using some kind of string identifier. You could use exceptions instead:

class AddressError < StandardError; end
class PermissionError < StandardError; end

def something
  bla_permission_invalid
  # do something
  true
end

def bla_permission_invalid
  raise AddressError if invalid_address?
  raise PermissionError if @user.not_one_of? [ :group1, :group2 ]
end

In the above code something calls bla_permission_invalid, performs its work and returns true. If an exception is raised in bla_permission_invalid, it automatically propagates up the call stack, you don't have to explicitly return it from something.

To handle the exception:

begin
  something
rescue AddressError
  # handle address error
rescue PermissionError
  # handle permission error
end

View more solutions

Comments

[Sergio Tulentsev]

You don't need the return here. And I find code blocks like this hard to read. But this will work.

[Sergio Tulentsev]

Where's "do something else" part? :)

[Sergio Tulentsev]

How are exceptions a part of OOP now? It's a totally unrelated concept ("error codes" vs. "exceptions")

[Boris Stitnicky]

@SergioTulentsev: Things are getting confused in my head already, and I didn't even open that Bowmore bottle yet. If I separate the statement by saying that: 1. "The original code asks too many questions, violating 'tell, don't ask' principle, which can be satisfied by OOPing more", and 2. "Mark Thomas is dead on in his observation that this is the case for exceptions", is it better? I guess not much. As a biologist, I'm simply not professional enough in programming, you CS majors out there, help me to express my feelings, please!

[Sergio Tulentsev]

Handling error codes is a totally valid approach (in languages that don't have exceptions). Here, I agree, exceptions seem to be a better choice. So, I'm gonna go ahead and remove reference to OOP, since it's irrelevant here.

[Boris Stitnicky]

And let me just comment, that raise has a popular alias fail, so instead of raise AddressError if invalid_address?, we can fail AddressError if invalid_address?.

[Artem P]

Is there way to replace return res if res because res is repeated here?

[Sergio Tulentsev]

@hlcs: off the top of my head - no.