Condition::Between crashes when calling against a nil

[dkatten]

Example:

rs = Ruy::RuleSet.new
rs.any do
    between :age, 10, 20
    eq :name, 'Steve'
end

rs.outcome "hello world"

rs.(age: 11) # => Works (prints "hello world)
rs.(age: 22, name: 'Jeff') # => Works (nil)
rs.(name: 'Jeff') # => ArgumentError: comparison of Fixnum with nil failed

If absent, the condition should return false (IMHO). Might also want to ensure that @from and @to are actually ordered correctly.

[snmgian]

We've been discussing how to handle nil values coming from the context, but didn't reach any agreement yet.

I don't support the idea of just evaluate the condistion as false.

Take, for example, greater_than_or_equal 18, :age evaluating to false because of :age being nil, someone would expect that less_than 18, :age will evaluate true.

If still considering no to crash when nil, why the condition should return false instead of true?

Also, I tihnk that everyone is not being surprised that 3 < nil will raise an exception, so why Ruy should behave differently?

Said that, I consider we need to raise friendlier exceptions.

[snmgian]

@migl has different opinions on this topic, I think his opinions are more closer to @dkatten than me :)

[migl]

I do feel like we need to reach an agreement on a convention of what the "nil" value means across the library. Even though Ruby encourages its developers to treat "nil" as just another value, I do feel like the purpose of a nil value is to express nullity, emptiness, void... you know what I mean. If Ruby doesn't encourage that convention, then maybe Ruy should.

Regardless of that, I convinced myself that comparing a number against the void should be an invalid operation, and an error should be risen. Friendlier exceptions are definitely something good to have.
To work around the case @dkatten mentions, we take advantage of the minimal evaluation approach of the all matcher, adding an except condition before the comparison is made. If the except condition does not match, the comparison won't be done.

all do
  except { eq nil, :age }
  less_than 18, :age
end

[migl]

I do know it's not the most intuitive way, so we should keep our eyes open to alternatives.
One from the top of my mind:

except nil, :age do
   less_than 18, :age
end

except would have two modes: one without arguments where the inner conditions are logically negated, and one with arguments that would by default check for equality to determine if it should or not perform the evaluation of the inner conditions.

[snmgian]

If users rely on the shortcut evaluation of the all matcher, Ruy will be unable to make some optimizations based on statistics. For example:

all do
  eq 'Mr.', :title
  less_than 18, :age
end

If the first matcher evaluates to false 50% of the times, and the second matcher evaluates to false 90% of the times, Ruy could reverse the evaluation order and achieve better evaluation performance.

That's an idea I've been thinking on ...

[migl]

So, if that optimization logic is in the plans, we should start thinking of a break condition like the one I proposed in my previous comment. It doesn't have to be THAT one, but there needs to be a way to put exceptions to evaluations.

We need to be careful not to end up reproducing Ruby's control statements. This should be a very specific syntax that applies to exceptions and nothing else.

[snmgian]

Closing this issue. Favoring the same behavior as Ruby. Informative exceptions is something that we'll take on.