What is the best way to handle nested conditionals in ruby?

Question

I have the following logic:

if something
  some_variable = true if another_variable
  your_variable = true if that_other_variable
elsif thatthing
  my_variable = true if another_variable
  her_variable = true if that_other variable
else
  puts "nothing found"
end

This is extremely simplified logic. Please forgive in advance for the names of the vars.

Coming from another language, I have conditioned my mind to think that nested inline if conditions are bad for readability reasons. I also have biases towards truthiness type tests. I tried to avoid negation if possible, but some times its unavoidable.

That being said, I have a couple of common possibilities of what I could use to replace the inline ifs, and one that is more uncommon that I put together to avoid negation. I am looking for feedback, and some things that you are doing to address this. Here it goes:

Possibility 1:

ternary:

if something
  some_variable = defined?(another_variable) ? true : false

Possibility 2:

negation:

if something
  some_variable = !defined?(another_variable)
# There are some variations to this, but this helps to address the negation I am trying to avoid.

Possibility 3 (I am favoring this one, but it seems a little uncommon):

if something
  some_variable = (defined?(another_variable) && another_variable.length > 0)    # Two truth based tests (does the var exist, and is it greater than 0 in length) This returns true/false.

Please let me know if anyone has any other solutions, and also which they feel is the most idiomatic.

Thank you!

Answer 1

Okay, instead of trying to stuff my answers in comments, I'll just give an answer, even though it's not quite an "answer".

All of your alternatives are not equivalent, they all do different things. What are you actually trying to do?

 some_variable = true if another_variable

Will set some_variable = true only if another_variable has a truthy value. In ruby, any value but nil or false is evaluated as true in a boolean test. another_variable has to already exist, otherwise you'll get an exception raised.

• So if another_variable exists, and is anything but nil or false, you'll set some_variable to true.
• If another_variable exists and is nil or false, you won't change the value of some_variable at all, it will remain unchanged. It will not be set to false. (if some_variable didn't already exist, it will be brought into existence with a nil value. Otherwise it's previous value will be unchanged).
• If another_variable didn't exist at all, the statement will raise an exception.

Is that what you intend? (And incidentally, using these meaningless variable names makes it a lot more confusing to talk about your example).

If that's really what you intend (and I'm doubting that it is), then there's probably no shorter or clearer way to do it than the way you've done: some_variable = true if another_variable.

There's nothing un-idiomatic with a one-line trailing if in ruby, it's perfectly idiomatic, people do it all the time, when it makes the code more clear. In your example -- and again it's hard to tell from your fake example -- I don't think it's the one-line if that makes it hard to tell what's going on, it is the weird structure of the top-level conditions, the overall structure of the code itself, it's very difficult to tell what the heck it's supposed to be doing. But you say you don't want to refactor that, so okay. And maybe that's just an artifact of you trying to produce a simple hypothetical and coming up with an awfully confusing one, maybe the original code isn't as confusing.

Your other alternative examples do something else, they are not equivalent. For instance:

some_variable = defined?(another_variable) ? true : false

This has different semantics than your first example.

• if another_variable exists at all, then some_variable will be set to true. Even if another_variable exists with the value nil or false. So long as another_variable exists, some_variable will be set to true.
• If another_variable does not exist at all some_variable will be set to false.
• There is no case where some_variable will be untouched.

If that's really what you intend to do, then there certainly is a more concise alternative. defined? returns nil if the variable is not defined, otherwise a string explaining the nature of the defined variable. If you want to turn that into strictly boolean true or false, you can use a double boolean negation: some_variable = !! defined?(another_variable). Recall that in ruby anything but false or nil is evaluated as truthy in a boolean context, so !! foo will turn into false if foo was nil or false, and true if foo was anything else.

People also just commonly do some_variable = defined?(another_variable) and figure that's good enough to capture "did another_variable exist", since some_variable will still be evaluated the same in a boolean test. Although really, in idiomatic ruby you only occasionally need defined? at all, mostly you are working with variables you already know exist, and you don't need to test to see if they do, and even more seldom do you need to store the results of that test in a variable.

Your additional examples all have yet different semantics again, none of them are actually equivalent. Hopefully I've given you the tools here to see what they all actually do. You can also try them all out in a little test script, or in the irb REPL.

I suspect neither of these are what you actually want to do. It depends on the actual semantics you want, which you haven't told us. If your first version you start out with actually does represent the semantics you want, then I think there's no shorter way to do it.

You say you are refactoring existing code -- Based on this example you give us, I think the existing code may likely be a mess, with lots of bugs in it, written strangely, and with lack of certainty about what it's even supposed to do, since you aren't the original developer. I'm guessing it doesn't have any tests either. In this situation, I'd start by writing some tests establishing what the code is supposed to do, and then you can try refactoring and make sure it still performs as expected. Without that, and with such messy code, and especially with being newish to ruby, your refactoring is highly likely to change the semantics of the program when you weren't intending to, likely introducing new and even stranger bugs.

Answer 2

You don't need that true if <var> part and can use !!instead:

if something
  some_variable = !!another_variable
  your_variable = !!that_other_variable
elsif thatthing
  my_variable  = !!another_variable
  her_variable = !!that_other variable
else
  puts "nothing found"
end