Nested conditional statements in Ruby

Question

My issues are two-fold:

1. I didn't realize I could nest multiple conditionals like this. It seems pretty nasty. I think I know what's going on here, but could someone who really gets it explain so I can really get the concept?

2. Since I don't understand nested conditionals too well, I'm a bit lost on the refactor, an area I am pretty weak in to begin with. Would y'all be so kind as to present some possible refactor solutions with explanations? It would help my limited understanding greatly.

---

def valid_triangle?(a, b, c, sum)
  if a != 0 || b != 0 || c != 0
    if a >= b
      largest = a
      sum += b
    else largest = b
      sum += a
    end
    if c > largest
      sum += largest
      largest = c
    else sum += c
    end
    if sum > largest
      return "true"
    else return "false"
    end
  else return false
  end
end

Answer 1

You can trim this down considerably by doing it a more Ruby-like way:

def valid_triangle?(*sides)
  case (sides.length)
  when 3
    sides.sort!

    sides[0] + sides[1] >= sides[2]
  else
    false
  end
end

Whenever possible, try and express your logic as a series of transformations on data, it's usually a lot easier to follow. In this case treat the incoming points as an array and sort them rather than having special-case logic for each point. That special case code is always trouble, hard to test, and prone to subtle failure if you make even a tiny mistake.

It's worth noting that in Ruby you should have your if statements formatted like this:

if condition
  # ...
elsif condition
  # ...
else
  # ...
end

Including code immediately following else may be valid syntax, but it's very confusing to look at. It seems you're testing against largest == b and have made a mistake, doing an assignment inadvertently, when really that's the content of the else block, not a condition for an elusive.

This code can be easily updated to include tests for negative lengths, like adding sides[0] > 0 as part of your logic.

Also, embrace the Ruby implicit return where the last value provided is the one returned from the method. The case statement actually propagates values from the one that triggered, making it very convenient for passing things through.