3
\$\begingroup\$

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 
\$\endgroup\$
2
  • 1
    \$\begingroup\$ The Stirling Approx. method should be validating that 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\$ Feb 23 '15 at 18:28
  • \$\begingroup\$ Math.gamma(171), it returns Infinity for larger values. There is also Math.lgamma \$\endgroup\$
    – steenslag
    Feb 27 '15 at 22:47
3
\$\begingroup\$
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

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.