Whats wrong with this if statement? Rails

Question

I am building a reputation system where users get points if milestones (10, 100, 1000, ...) are archieved. I have this if statement line:

if (before_points < ((10 || 100 || 1000 || 10000 || 100000 || 1000000))) && (after_points >= ((10 || 100 || 1000 || 10000 || 100000 || 1000000)))

It should return true if the points where either less than 10 or 100 or 1000 ...before, and if the points were more or equal to either 10 or 100 or 1000 ... afterwards.

It works if it was below 10 before, and more than 10 afterwards, and I am not quite sure if it works with 100, but it doesnt work if the points were below 1000 before and more than 1000 afterwards.

Is this the correct way to do this? Is it better to do this with a switch/case?

Answer 1

That's not really how logic operation work. The statement:

(10 || 100 || 1000 || 10000 || 100000 || 1000000)

will evaluate to 10. The || operator between 2 or more numbers will return first non-nil value, in this case that's 10, the first value. Related question.

And even if that weren't the case, if the before_points < 10 is true, the before_points < 1000000 would also be true and if only before_points < 1000000 was true, the if statement would still execute just the same as with before_points < 10, so the logic would be wrong.

Depending on what you want to solve, you could either use case or define your milestones in array and iterate values 10,100,...,1000000, setting new milestone each time the condition is still true.

Answer 2

A more compact way you could do it...

[10, 100, 1000, 10000, 100000, 1000000].any?{|n| before_points < n && after_points >= n}

That expression will return true if a boundary is crossed, or false otherwise

Answer 3

Your assumption is wrong.

if (before_points < ((10 || 100 || ...

will first evaluate the part

10 || 100

which will always return 10 because 10 evaluates to truthy, hence this line

if (before_points < ((10 || 100 || 1000 || 10000 || 100000 || 1000000))) && (after_points >= ((10 || 100 || 1000 || 10000 || 100000 || 1000000)))

is effectively the same of

if (before_points < 10) && (after_points >= 10)

I'm not sure what you want to achieve, but it's probably better to use a case (this is just an example)

case
  when before_points < 10 && after_points >= 10
  # ...
  when before_points < 100 && after_points >= 100
  # ...
  else
  # ...
end