def divisible?(n)
if n % 1 == 0 &&
n % 2 == 0 &&
n % 3 == 0 &&
n % 4 == 0 &&
n % 5 == 0 &&
n % 6 == 0 &&
n % 7 == 0 &&
n % 8 == 0 &&
n % 9 == 0 &&
n % 10 == 0 &&
n % 11 == 0 &&
n % 12 == 0 &&
n % 13 == 0 &&
n % 14 == 0 &&
n % 15 == 0 &&
n % 16 == 0 &&
n % 17 == 0 &&
n % 18 == 0 &&
n % 19 == 0 &&
n % 20 == 0
return true
else
return false
end
end
\$\begingroup\$
\$\endgroup\$
Add a comment
|
2 Answers
\$\begingroup\$
\$\endgroup\$
2
Ruby refactor: use Enumerable.all?:
def divisible?(n)
(1..20).all? { |x| n % x == 0 }
end
Mathematical refactor:calculate the least common multiple of the integers in the range (Integer#lcm is available from Ruby 1.9):
def divisible?(n)
n % (1..20).reduce(:lcm) == 0
end
This second snippet is, of course, more efficient once you pre-calculate (1..20).reduce(:lcm) only once and store it somewhere.
-
\$\begingroup\$ Would the first example benefit from 20..1 rather than 1..20? Less values of n are divisible by the higher divisors, leading to faster exit. Checking for factors seems like a case where premature optimisation is reasonable. (I don't know Ruby, maybe 1..20 is not equivalent to the big series of ifs in the question?) \$\endgroup\$– SeanFeb 4, 2013 at 22:01
-
\$\begingroup\$ @Sean: yeah, good point, you could write
(1..20).reverse_each.all? .... Anyway, one you've decided to do it in a mathematically sound way you might as well go for the second snippet, which is simple enough to understand and much faster. \$\endgroup\$– toklandFeb 4, 2013 at 22:14
\$\begingroup\$
\$\endgroup\$
Note that you're doing this, which is a pretty egrigious use of if/else:
if boolean
return true
else
return false
end
If the branches of your if/else are return true and return false, you should just return the condition you're testing! In Ruby, you don't even need return, just let the condition "fall off" the end of the method.
Ignoring the ability to clean up your condition itself, a first pass at cleaning up your function could be:
def divisible?(n)
n % 1 == 0 &&
n % 2 == 0 &&
n % 3 == 0 &&
n % 4 == 0 &&
n % 5 == 0 &&
n % 6 == 0 &&
n % 7 == 0 &&
n % 8 == 0 &&
n % 9 == 0 &&
n % 10 == 0 &&
n % 11 == 0 &&
n % 12 == 0 &&
n % 13 == 0 &&
n % 14 == 0 &&
n % 15 == 0 &&
n % 16 == 0 &&
n % 17 == 0 &&
n % 18 == 0 &&
n % 19 == 0 &&
n % 20 == 0
end
