CODEWITHSUNDEEP
rubyalgorithmfunctionif-statement
Huy Tran
Huy Tran

Reputation: 1902

Ruby one-liner and warning: 'else without rescue is useless'

I'm trying to print a basic triangle pattern with a Ruby function which only accepts positive odd number for n 

def triangle(n, chars)
    puts "#{n} must be odd and positive" if n % 2 == 0 || n < 0 else (n + 1).times { |i| puts chars * i } 
end

The problem is this function accepts not only odd but also positive n. triangle(3, '#')would print 

    #
    ##
    ###

but 'triangle(4, '#')' also works fine 

    #
    ##
    ###
    ####

It seems my if statement is not working correctly and I get the warning else without rescue is useless. Why and how should I fix this?

Upvotes: 1

Views: 1444

Answers (3)

engineersmnky
engineersmnky

Reputation: 29478

As a more graceful implementation lets take this a step further and handle unexpected arguments 

def triangle(n, chars='#')
  raise(ArgumentError, "{n} must be odd and positive") unless n.to_i.odd? && n.to_i > 0
  1.upto(n.to_i) { |i| chars.to_s * i} 
end

This will now handle any n that responds to to_i and any chars that responds to to_s. e.g. '13', ['x', 'x', 1] are still valid arguments even if they make a weird triangle.

Ruby is extremely supportive of duck typing in this fashion and it makes your code less type specific.

Upvotes: 0

Schwern
Schwern

Reputation: 165218

Ruby is interpreting your code as two separate statements.

puts "#{n} must be odd and positive" if n % 2 == 0 || n < 0

else (n + 1).times { |i| puts chars * i }

The else is not associated with the if. Ruby is, I guess in desperation, interpreting that as part of a begin/rescue/else condition. Why it's not a syntax error, I have no idea, but it's interpreting it as having no begin block which is technically "successful" so the else always runs.

Conditional statements like do this if that are intended only for simple conditions covering simple statements. Trying to wedge in an else condition is right out. Instead, use a normal condition.

def triangle(n, chars)
    if n % 2 == 0 || n < 0
        puts "#{n} must be odd and positive"    
    else
        (n + 1).times { |i| puts chars * i } 
    end
end

In general, avoid the else entirely by taking care of the error condition immediately. It avoids nesting the bulk of the function in an else block. This is more relevant for longer functions, but it's a good habit to get into.

def triangle(n, chars)
    if n % 2 == 0 || n < 0
        puts "#{n} must be odd and positive"
        return nil
    end

    (n + 1).times { |i| puts chars * i } 
end

Finally, errors should be handled with exceptions, not printed error messages. Exceptions can be caught and dealt with by the caller, printed error messages are difficult to catch and bubble up to the user. Exceptions, if not dealt with, halt the program; if the user forgets to handle an error the program will stop and they'll know to fix it. Printed error messages simply let the program plow forward and can be ignored leading to further problems down the road.

Exceptions can also be categorized allowing the caller to figure out what sort of error happened and act appropriately.

def triangle(n, chars)
    if n % 2 == 0 || n < 0
        raise ArgumentError, "#{n} must be odd and positive"
    end

    (n + 1).times { |i| puts chars * i } 
end

Upvotes: 5

moveson
moveson

Reputation: 5213

The problem seems to be the flow control is getting confused with the logic as to what is output. This will do what you want, I think:

def triangle(n, chars)
  if (n % 2 == 0) || (n < 0)
    puts "#{n} must be odd and positive"
  else
    (n + 1).times { |i| puts chars * i }
  end
end

Upvotes: 2

Related Questions