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

The Odd-Even Challenge

Submitted by: @import:stackexchange-codereview··
0
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:

  • 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 False


What 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 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.