Em Sta
Em Sta

Reputation: 1696

Why does my code only select year that is equal 10?

I'm a beginner in programming and have problems with this if statement:

if (f.year == (10 || 20 || 30 || 40 || 50 || 60 || 70 || 80 || 90 || 100 || 110 || 120)) && (f.rund != true)

The first problem is that this code is very complicated. Actually I only want to check if the f.year is a round two-digit number.

Next my code does not work correctly. Somehow it only selects the f.year that are equal 10.

How can I solve these problems?

Upvotes: 2

Views: 190

Answers (7)

Marek Lipka
Marek Lipka

Reputation: 51151

It's because

(10 || 20 || 30 || 40 || 50 || 60 || 70 || 80 || 90 || 100 || 110 || 120)

expression always evaluates to 10.

You can solve the problem with, for example:

(1..12).map { |el| el * 10 }.include?(f.year)

or, as suggested by @AurpRakshit:

(1..12).map(&10.method(:*)).include?(f.year)

Here you have more examples of generating this kind of array.

Or, if you really want to check if f.year is round two-digit number, you can:

(10...100).include?(f.year) && f.year % 1 == 0

Upvotes: 4

toro2k
toro2k

Reputation: 19230

You are already told why your code doesn't work as expected, I'm answering just to suggest to use a mathematical approach here instead of using include?, your condition could be written as:

if f.year.modulo(10).zero? && f.year.between?(10, 120) && !f.rund
  ...

It may be a little less clear but it is much faster.


Update

The drawback of this solution is that it fails when f.year is not a Numeric object:

nil.modulo(10)
# NoMethodError: ...

While:

[10].include?(nil)
# => false

The benchmarck:

require 'fruity'

a = (1..10000)

compare do   
  map_include do
    a.each do |i|
      (1..12).map(&10.method(:*)).include?(i)
    end
  end

  step_include do
    a.each do |i|
      (10..120).step(10).include?(i)
    end
  end

  divmod_include do
    a.each do |i|
      q, r = i.divmod(10); (1..12).include?(q) && r.zero?
    end
  end

  math do
    a.each do |i|
      i.modulo(10).zero? && i.between?(10, 120)
    end
  end      
end
Running each test once. Test will take about 2 seconds.
math is faster than divmod_include by 1.9x ± 0.01
divmod_include is faster than step_include by 9x ± 0.1
step_include is faster than map_include by 3.4x ± 0.1

Upvotes: 2

the Tin Man
the Tin Man

Reputation: 160551

It's hard to tell what the OP wants but...

require 'fruity'

ARY = (1..1000).to_a

compare do
  test_mod_and_le do
    ARY.each do |i|
      (i % 10 == 0) && (i <= 120)
    end
  end

  test_mod_and_range do
    ARY.each do |i|
      (i % 10 == 0) && ((10..120) === i)
    end
  end

  test_case_when do
    ARY.each do |i|
      case i
      when 10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120
        true
      else
        false
      end
    end
  end

  map_include do
    ARY.each do |i|
      (1..12).map(&10.method(:*)).include?(i)
    end
  end

  step_include do
    ARY.each do |i|
      (10..120).step(10).include?(i)
    end
  end

  divmod_include do
    ARY.each do |i|
      q, r = i.divmod(10); (1..12).include?(q) && r.zero?
    end
  end

  math do
    ARY.each do |i|
      i.modulo(10).zero? && i.between?(10, 120)
    end
  end      
end

Which outputs:

Running each test 32 times. Test will take about 4 seconds.
test_case_when is similar to test_mod_and_le
test_mod_and_le is faster than test_mod_and_range by 19.999999999999996% ± 10.0%
test_mod_and_range is faster than math by 50.0% ± 10.0%
math is faster than divmod_include by 80.0% ± 10.0%
divmod_include is faster than step_include by 5.9x ± 0.1
step_include is faster than map_include by 2.9x ± 0.1

Upvotes: 1

Stefan
Stefan

Reputation: 114138

You can use Range#step or Numeric#step:

(10..120).step(10).to_a  #=> [10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120]

10.step(120, 10).to_a    #=> [10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120]

And call Enumerable#include?:

(10..120).step(10).include? year

10.step(120, 10).include? year

Upvotes: 4

Ross McNab
Ross McNab

Reputation: 11567

To answer your first point, the code should read:

if (f.year == 10 || f.year == 20 || f.year == 30 ...

Your expression f.year == (10 || 20 || 30 ... doesn't work, because it is evaluated by ruby as follows:

  • The brackets force 10 || 20 || 30 ... to be evaluated first
  • The || operator returns its left operand if it is true, otherwise it returns its right operand
  • Ruby considers anything that isn't nil or false to be "true", so the expression 10 || 20 || 30 ... evaluates to 10
  • So your expression boils down to (f.year == 10) && (f.rund != true)

Upvotes: 2

KappaNossi
KappaNossi

Reputation: 2706

Conditions do not work this way. All numbers within the brackets, connected with OR are evaluated before checking the equality with f.year.

Most answers here seem overcomplicated. You can use basic math to solve your issue:

if year % 10 == 0 && year.to_s.size == 2
   # do stuff
end

The modulo operator % returns the remainder when dividing by 10 in this example. If the remainder is 0, it's a multiple of 10. You can use any number. Modulo 2 would check whether the number is even.

The second part checks the number of digits. It converts it to a string first with to_s and then checks the length of it, basically how many characters are in there. Converting 10 to string results in '10' which has 2 characters.

Your question seems a bit unclear. Do you want to include the numbers 100, 110 and 120 like in your code example? Or do you want only two digit numbers like stated in your text?

Upvotes: 0

sawa
sawa

Reputation: 168081

I am not sure about your question, but the first condition can be written as

q, r = f.year.divmod(10); (1..12).include?(q) && r.zero?

or

[10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120].include?(f.year)

Upvotes: 1

Related Questions