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

An implementation for the double factorial

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

Problem

I have written this piece of code that implements the double factorial in Python both iteratively and recursively; the code works without problems, but I'm interested in improving my overall programming style. Here's the code:

def semif_r(n):                 #recursive implementation
    if n == 0 or n == 1:
        z = 1
    else:
        z= n * semif_r(n-2)
    return z

def semif_i(n):             #iterative implementation
    N = 1
    if n == 0 or n == 1:
        return 1
    elif n%2 == 1:
        for i in range(0,n/2):
            N =  (2*i + 1)*N
            VAL = N
        return n*VAL

    elif n%2 == 0:
        for i in range(0,n/2):
            N =  (2*i+2)*N
            VAL = N
    return VAL


I hope that some experienced programmers can give me some feedback about improving my code!

Solution

The recursive solution

The z local variable, you can just return directly.

The else could be dropped, as the if before it returns from the function.

Adding some doctests would be useful.

def semif_r(n):
    """
    >>> semif_r(5)
    15
    >>> semif_r(6)
    48
    >>> semif_r(25)
    7905853580625
    """
    if n == 0 or n == 1:
        return 1
    return n * semif_r(n - 2)


The iterative solution

No need to declare N up front. You could declare and initialize when you need it.

VAL is unnecessary, whenever its value is referenced, it's equal to N, so you could use N instead of VAL everywhere.

The start value of a range is 0 by default, so you don't need to specify that explicitly.

In n / 2, you're counting on that integer division will occur with truncation. In Python 3 there is a separate operator // for this.
It's good to make your code ready for Python 3, by using //, and adding this import:

from __future__ import division


The logic in the two range loops are almost the same. It would be good to extract that logic to a helper function.

As with the recursive implementation, doctests would be nice.

Note that in this code:

elif n%2 == 1:
    # ...

elif n%2 == 0:
    # ...


There, the 2nd elif could be replaced with a simple else,
since if n % 2 != 1, then it must be inevitably 0.

Putting it all together:

def semif_i(n):  
    """
    >>> semif_i(5)
    15
    >>> semif_i(6)
    48
    >>> semif_i(25)
    7905853580625
    """
    if n == 0 or n == 1:
        return 1

    def multiply(coef):
        val = 1
        for i in range(n // 2):
            val *= (2 * i + coef)
        return val

    if n % 2 == 1:
        return n * multiply(1)

    return multiply(2)


Coding style

Please follow PEP8, the coding style guide (see the writing style in my sample implementations).

Code Snippets

def semif_r(n):
    """
    >>> semif_r(5)
    15
    >>> semif_r(6)
    48
    >>> semif_r(25)
    7905853580625
    """
    if n == 0 or n == 1:
        return 1
    return n * semif_r(n - 2)
from __future__ import division
elif n%2 == 1:
    # ...

elif n%2 == 0:
    # ...
def semif_i(n):  
    """
    >>> semif_i(5)
    15
    >>> semif_i(6)
    48
    >>> semif_i(25)
    7905853580625
    """
    if n == 0 or n == 1:
        return 1

    def multiply(coef):
        val = 1
        for i in range(n // 2):
            val *= (2 * i + coef)
        return val

    if n % 2 == 1:
        return n * multiply(1)

    return multiply(2)

Context

StackExchange Code Review Q#111433, answer score: 4

Revisions (0)

No revisions yet.