CODEWITHSUNDEEP
ruby
user4627062
user4627062

Reputation:

'if not or' not working in Ruby

I have this code in Ruby:

if @pass.nil?
        badpass(@nick)
elsif not clt.correctpassword?(@nick, @pass)
        badpass(@nick)

I wanted to condense it to a single line so I tried writing it as:

if not clt.correctpassword?(@nick, @pass) or @pass.nil?
        badpass(@nick)

But then badpass(@nick) doesn't get triggered for either cases, whereas it works in both cases in the top example.

Why does this happen?

Upvotes: 0

Views: 130

Answers (2)

Michael Geary
Michael Geary

Reputation: 28870

In your revised code the tests are in the opposite order from the original. It's as if you had written:

if not clt.correctpassword?( @nick, @pass )
    badpass( @nick )
elsif @pass.nil?
    badpass( @nick )
end

Put the tests in the same order as the original and it will work like the original:

if @pass.nil? or not clt.correctpassword?( @nick, @pass )
    badpass( @nick )
end

Remember that Ruby, like many other languages, uses short circuit evaluation for the logical operators and, or, && and ||. This means that it evaluates the expression to the left of the operator first, and then only evaluates the expression to the right of the operator if it needs to.

BTW, many Rubyists would write it this way instead:

badpass( @nick ) if @pass.nil? or not clt.correctpassword?( @nick, @pass )

That does the same thing; it's just a matter of which style you prefer.

If the .correctpassword? method is code that you can modify, then another nice option would be to add a nil test at the beginning of that method. In fact, you may as well test for nil on both nick and pass:

def correctpassword?( nick, pass )
    return false if nick.nil? or pass.nil?
    ... the rest of your code here ...
end

Then when you call this method you can simply use:

if not clt.correctpassword?( @nick, @pass )
    badpass( @nick )
end

or this simple code:

badpass( @nick ) unless clt.correctpassword?( @nick, @pass )

Upvotes: 4

spickermann
spickermann

Reputation: 106972

You should not use and, or or not in conditions. They are for control flow and they offen lead to precedence problems in conditions.

Let's transform your code:

if @pass.nil?
  badpass(@nick)
elsif not clt.correctpassword?(@nick, @pass)
  badpass(@nick)
end

Make it more Ruby-like:

if !@pass
  badpass(@nick)
elsif !clt.correctpassword?(@nick, @pass)
  badpass(@nick)
end

Combine conditions:

if !@pass || !clt.correctpassword?(@nick, @pass)
  badpass(@nick)
end

Simplefy condition (De Morgan's law)

if !(@pass && clt.correctpassword?(@nick, @pass))
  badpass(@nick)
end

Or even more Ruby-like:

badpass(@nick) unless @pass && clt.correctpassword?(@nick, @pass)

And I agree with @MichaelGeary that you should move the check for the existence of @pass into the correctpassword? method...

Upvotes: 3

Related Questions