patternpythonMinor
An implementation for the double factorial
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:
I hope that some experienced programmers can give me some feedback about improving my 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 VALI hope that some experienced programmers can give me some feedback about improving my code!
Solution
The recursive solution
The
The
Adding some doctests would be useful.
The iterative solution
No need to declare
The start value of a range is 0 by default, so you don't need to specify that explicitly.
In
It's good to make your code ready for Python 3, by using
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:
There, the 2nd
since if
Putting it all together:
Coding style
Please follow PEP8, the coding style guide (see the writing style in my sample implementations).
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 divisionThe 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 divisionelif 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.