I'm new to Ruby and as an exercise I implemented different factorial algorithms. Other rules are: raise an ArgumentError exception if n < 0 or if n is not an integer.
I'm interested in whatever feedback you can think of, including Ruby idioms (can something be implemented more idiomatic?). If you see anything that can be improved in any way, let me know.
# While loop
def factorial_w(n)
validate(n)
total = 1
while n > 1
total *= n
n -= 1
end
total
end
# For loop
def factorial_f(n)
validate(n)
total = 1
for i in (2 .. n)
total *= i
end
total
end
# Recursive
def factorial_r_recursion(n)
if n < 2
1
else
n * factorial_r(n - 1)
end
end
def factorial_r(n)
validate(n)
factorial_r_recursion(n)
end
# Tail-recursive
def factorial_tr_recursion(n, acc)
if n < 2
acc
else
factorial_tr(n - 1, n * acc)
end
end
def factorial_tr(n, acc = 1)
validate(n)
factorial_tr_recursion(n, acc)
end
# Iterative with Range.each
def factorial_re(n)
validate(n)
if n == 0
1
else
(2 .. n - 1).each {|i| n *= i}
n
end
end
# Iterative with Range.inject
def factorial_ri(n)
validate(n)
(1 .. n).inject(:*) || 1 # || 1 handles n = 0 so !0 = 1
end
# Iterative with Range.reduce
def factorial_rr(n)
validate(n)
(1 .. n).reduce(:*) || 1 # || 1 handles n = 0 so !0 = 1
end
# Stirling's Approximation, will return infinity if n > 170
def factorial_sa(n)
validate(n)
Math.sqrt(2 * Math::PI * n) * (n / Math::E) ** n
end
# Helper for argument validation
def validate(n)
if n < 0 or not n.kind_of? Integer
raise ArgumentError, "n must be an integer and >= 0"
end
end
1 Answer 1
if n < 2
1
else
n * factorial_r(n - 1)
end
This is exactly equivalent to the following, which is called a ternary statement:
n < 2 ? 1 : n * factorial_r(n - 1)
Your factorial_r
and factorial_r_recursion
functions call each other, but this makes you call your validate function over and over. You should rewrite it so that factorial_r_recursion only ever calls itself, because you can ensure that everything it produces is valid already. (Same with the tr
functions)
if n < 0 or not n.kind_of? Integer
This would be much better worded as:
unless n.is_a? Integer && n >= 0
No one uses kind_of?
, and you need to check if the number is an Integer before you compare it to 0.
Although it doesn't appear to be in the scope of your project, ideally you would not call validate
at the beginning of each factorial method. You would call it once, when you actually get n
, and THEN pass it in to the factorial function.
Some last points; I recognize that this is a learning step for you, but do not use for
loops in ruby. They have bad side effects and you do not need them, there are plenty of ways to iterate over collections. Also the while
loop looks rather ugly but that's because it's the wrong tool for the job, so it's to be expected.
All in all, it's pretty decent ruby! Good job on using reduce / inject, a lot of beginners don't use/comprehend what they do., however reduce
is an alias of inject
so to have them both is kind of silly :P
n <= 170
, if anyone can think of a good way to incorporate that (perhaps send something into the validate function as an optional param?) that would be another good answer \$\endgroup\$Math.gamma(171)
, it returns Infinity for larger values. There is also Math.lgamma \$\endgroup\$