HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Simplifying this factorial function

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisfactorialfunctionsimplifying

Problem

def factorial(t,n):
    if n == 1 : return t

    else: x = (t * (n-1))

    n = n-1

    return factorial(x,n)

print factorial(6,6)


I can't seem to figure out a way to just stick to requiring one parameter input while keeping the program small. I also want the function to remain recursive (trying to work on my recursive thinking).

If you have any other cooler simplifications or other fancy methods, please show it off.

Solution

First, if n == 1the method returns, so there is no need for the else.

Let us see what 6! would result in: 6 5! which is just 6 5 * 4! and so on.

Now we see a patter: n! = n * (n-1)!

Keeping this in mind we can refactor the given method to only take one parameter and to be recursive.

def factorial(n):
    if n < 0 :
        raise ValueError("Negative values are not allowed.")
    if n == 0 : return 1

    return n * factorial(n-1)

print factorial(6)


As James Snell stated correctly in his comment, we can replace the n == 1 check with a n = 1.

As jonrsharpe stated correctly in his comment,
0! should return 1` and also negative numbers don't have a factorial.

As Simon André Forsberg stated correctly in his comment, rasing an error for a negative parameter should be done.

Code Snippets

def factorial(n):
    if n < 0 :
        raise ValueError("Negative values are not allowed.")
    if n == 0 : return 1

    return n * factorial(n-1)

print factorial(6)

Context

StackExchange Code Review Q#68981, answer score: 8

Revisions (0)

No revisions yet.