From: tamouse mailing lists <tamouse.lists@...>
Date: 2012-12-04T12:21:26+09:00
Subject: Re: If statement

On Sun, Dec 2, 2012 at 4:43 PM, Masoud Ahmadi <lists@ruby-forum.com> wrote:
> Will anyone be able to point out what I am doing wrong.
> The code does not understand that I have said Q
>
>     loop {
>       puts("give me the command to execute or press Q to quit")
>       op=gets
>       if "#{op}" == "Q"
>         then
>         Kernel.exit(0)
>       else
>         value = `#{op}`
>         puts "#{value}"
>       end
>     }
>
> --
> Posted via http://www.ruby-forum.com/.

While others have addressed some of the syntax problems, I'd like to
address the overall approach.

In a "try something out, see how it works" approach, this is great.
But there are better ways to do something like this.

There are times when rolling a non-terminating loop with a mid-point
exit makes sense. However, there isn't much need to call the exit
method to do that.


loop do

break if condition

end

is the typical way to do this. This leaves your program a place to do
any sort of end clean up you might like to do, and so on. This sort of
thing is nice in the scenario you've devised at it most cleanly limits
the prompt, setting of the variable and test for completion once each
inside the loop, unlike while and unless will in this case.

If you'll permit, and this is in no way meant to say you're wrong in
your approach, just an offer, this is how I'd approach such a thing:

loop do
  puts("give me the command to execute or enter Q to quit")
  op = gets.strip   # the reason for strip was given above
  break if op == 'Q'
  value = system op ## HOLY KRAKEN ARE YOU SURE YOU WANT TO DO THIS???
  puts value
end

The point here is that calling the Kernel.exit(0) method makes the
code look a lot more complex that it is. Reserve the use of exit for
times when you want your code to exit early and report an error
(non-zero exit status) and then only if you've gotten yourself into a
place you don't really want to unwind, or it's just more clear to the
code reader what should be done. (Kinda like you should never end a
sentence with a preposition, but that is something with which one
aught not put up.) (Apologies, Churchill witicism.)

The other thing I'd like to point at is the if modifier -- these are
really nice little features in ruby (other languages have them, too)
that make the above very clean, to my eye. (Others will disagree, of
course.) But as you have this construction:

>       if "#{op}" == "Q"
>         then
>         Kernel.exit(0)
>       else
>         value = `#{op}`
>         puts "#{value}"
>       end

I just want to point out that the "then" is not needed at all, nor is
the else. Calling exit at that point *exits* the program immediately
-- there is no else possible. Shortening what you have a bit:

if op == "Q"
  Kernel.exit(0)
end

value = ... (and the rest)

is sufficient; Placing it in an else clause is overkill (and could
actually cause someone to overlook something if they were maintaining
such a construction).

Joel mentioned the issue of trusting users *not* to do something
malicious with thi (this DOES include yourself! -- mistakes do
happen). Something learn very early is never to trust user (or any)
input. I assume this is just a practice piece, which is fine.

Given the nature of this snippet, I'm assuming you are a beginning
programming? This is great! I hope you take my response in the manner
intended: encouraging, showing a little beyond the basic question "why
doesn't it work". Keep at it!