A more elegant solution to Ruby Koans' triangle.rb

Question

I have been working through Ruby Koans and made it to about_triangle_project.rb in which you are required to write the code for a method, triangle.

Code for these items are found here:

https://github.com/edgecase/ruby_koans/blob/master/koans/about_triangle_project.rb

https://github.com/edgecase/ruby_koans/blob/master/koans/triangle.rb

In triangle.rb, I created the following method:

def triangle(a, b, c)
  if ((a == b) && (a == c) && (b == c))
    return :equilateral
  elsif ((a == b) || (a == c) || (b == c))
    return :isosceles
  else
    return :scalene
  end
end

I know from reading Chris Pine's "Learn to Program" there is always more than one way to do things. Although the above code works, I can't help but think there is a more elegant way of doing this. Would anyone out there be willing to offer their thoughts on how they might make such a method more efficient and compact?

Another thing I am curious about is why, for determining an equilateral triangle, I was unable to create the condition of (a == b == c). It is the proof for an equilateral triangle but Ruby hates the syntax. Is there an easy explanation as to why this is?

Answer 1

There is an easy explanation for why that is:

== in Ruby is an operator, which performs a specific function. Operators have rules for determining what order they're applied in — so, for example, a + 2 == 3 evaluates the addition before the equality check. But only one operator at a time is evaluated. It doesn't make sense to have two equality checks next to each other, because an equality check evaluates to true or false. Some languages allow this, but it still doesn't work right, because then you'd be evaluating true == c if a and b were equal, which is obviously not true even if a == b == c in mathematical terms.

As for a more elegant solution:

case [a,b,c].uniq.size
when 1 then :equilateral
when 2 then :isosceles
else        :scalene
end

Or, even briefer (but less readable):

[:equilateral, :isosceles, :scalene].fetch([a,b,c].uniq.size - 1)

Answer 2

Another approach:

def triangle(a, b, c)
  a, b, c = [a, b, c].sort
  raise TriangleError if a <= 0 or a + b <= c
  return :equilateral if a == c
  return :isosceles if a == b or b == c
  return :scalene
end

Answer 3

I borrowed Chuck's cool uniq.size technique and worked it into an oo solution. Originally I just wanted to extract the argument validation as a guard clause to maintain single responsibility principle, but since both methods were operating on the same data, I thought they belonged together in an object.

# for compatibility with the tests
def triangle(a, b, c)
  t = Triangle.new(a, b, c)
  return t.type
end

class Triangle
  def initialize(a, b, c)
    @sides = [a, b, c].sort
    guard_against_invalid_lengths
  end

  def type
    case @sides.uniq.size
    when 1 then :equilateral
    when 2 then :isosceles
    else :scalene
    end
  end

  private
  def guard_against_invalid_lengths
    if @sides.any? { |x| x <= 0 }
      raise TriangleError, "Sides must be greater than 0"
    end

    if @sides[0] + @sides[1] <= @sides[2]
      raise TriangleError, "Not valid triangle lengths"    
    end
  end
end