patternpythonMinor
The Odd-Even Challenge
Viewed 0 times
eventheoddchallenge
Problem
After posting my first question on here, I got a lot of great feedback. I've decided to try another challenge and write it as cleanly as possible.
The rules of Odd-Even consists of the following tasks:
Given a max number a string should be returned counting up to and including that max number and adhere to the following rules in order:
I came up with the following code:
What does not sit well with me is the `parse_o
The rules of Odd-Even consists of the following tasks:
Given a max number a string should be returned counting up to and including that max number and adhere to the following rules in order:
- Print "
Even" instead of number, if the number is even, which means it is divisible by 2.
- Print "
Odd" instead of number, if the number is odd, which means it is not divisible by 2 and is not a prime (it should be a composite).
- Print the number if it does not meet above two conditions.
I came up with the following code:
def create_oddeven_string(max_number):
"""Returns a string with numbers up to and including max_number."""
return ",".join(create_oddeven_list(max_number))
def create_oddeven_list(max_number):
"""Returns a list with string representations of oddeven parsed numbers up to and
including the max_number."""
return map(parse_oddeven_number, xrange(1, max_number+1))
def parse_oddeven_number(number):
"""Returns the string "Even" when number is even, "Odd" when number is odd
and composite, or the string representation of the number if prime."""
if is_divisable(number, 2):
return "Even"
elif is_composite(number):
return "Odd"
else:
return str(number)
def is_divisable(number, modulo):
"""Returns True if number % modulo == 0."""
return number % modulo == 0
def is_composite(number):
"""Returns True if number is not a prime number (only divisable by itself and 1)."""
if number <= 3:
return False
elif is_divisable(number, 2) or is_divisable(number, 3):
return True
else:
i = 5
while i*i <= number:
if is_divisable(number, i) or is_divisable(number, (i+2)):
return True
i = i+6
return FalseWhat does not sit well with me is the `parse_o
Solution
Your code looks good and is well documented.
A few details :
-
instead of having
-
I wouldn't define a
-
Because you know an upper bound for the number you'll consider during primality check, it may be a good idea to use the Sieve of Eratosthenes.
A few details :
-
instead of having
is_composite function, I'd rather have a is_prime function : it is more common and it reuses the terms used to describe the problem. (You just have to swap True/False).-
I wouldn't define a
is_divisable function but it may be a good idea if you think it is necessary. However, the proper English word is "divisible" and not "divisable". Also, I wouldn't call the second argument "modulo" to avoid mixing argument name and operator name. Wikipedia suggests "modulus". Finally, the docstring should tell more about what is intended and less about how it is implemented. """Returns whether number is divisable by modulus""".-
Because you know an upper bound for the number you'll consider during primality check, it may be a good idea to use the Sieve of Eratosthenes.
Context
StackExchange Code Review Q#109742, answer score: 3
Revisions (0)
No revisions yet.