The Odd-Even Challenge

Question

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_oddeven_number(number) function. If new situations arrive then that list is going to be a long if, elif, else structure that I'd rather not have. But I don't know what I do about simplifying that function.

SylvainD · Accepted Answer · 2015-11-04 11:01:02Z

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.

200_success · Accepted Answer · 2015-11-05 07:56:23Z

Your code is very readable. However, I think that some of your one-line functions may be overkill. For example, the cost of defining is_divisable(number, modulo) (which should be spelled is_divisible(…), by the way) outweighs the benefit, in my opinion.

parse_oddeven_number() is misnamed. Parsing means to analyze a string and try to make sense of it. The function doesn't parse anything. classify_number() would be a more appropriate name.

A deeper problem, though, is that the algorithm is inefficient. Trial division is fine for testing whether some number is prime. However, if you want to test a sequence of numbers, you would be much better off with a sieve-of-eratosthenes.

An implementation of it is short enough to be just one function or two. I'd break it up into two functions to provide the flexibility to format the results differently.

def odd_even_list(max_number):
    sieve = range(max_number + 1)       # In Python 3, use list(range(...))
    for even in xrange(2, max_number + 1, 2):
        sieve[even] = "Even"
    for odd in xrange(3, max_number + 1, 2):
        if isinstance(sieve[odd], int):
            # Found a prime.  Mark all odd multiples of it.
            for composite in xrange(3 * odd, max_number + 1, 2 * odd):
                sieve[composite] = "Odd"
    sieve.pop(0)
    return sieve

def odd_even_string(max_number):
    return ",".join(str(entry) for entry in odd_even_list(max_number))

jonrsharpe · Accepted Answer · 2015-11-04 11:05:29Z

In terms of making parse_oddeven_number more flexible, you could do something like:

is_even = lambda number: is_divisible(number, 2)

CLASSIFIERS = [
    (is_even, 'Even'),
    (is_composite, 'Odd'),
]

def classify_number(number, classifiers):
    """Classify a number according to the supplied rules."""
    for classifier, description in classifiers:
        if classifier(number):
            return description
    return str(number)

def parse_oddeven_number(number):
    """Whether the number is even, composite or neither."""
    return classify_number(number, CLASSIFIERS)

If you come up with new classifiers, or want to use a completely different list for some other purpose, you can now easily do so.

Stack Exchange Network

The Odd-Even Challenge

3 Answers 3

You must log in to answer this question.

Linked

Hot Network Questions

The Odd-Even Challenge

3 Answers 3

You must log in to answer this question.

Linked

Related

Hot Network Questions