4
\$\begingroup\$

I completed this Codewars exercise. Here are the instructions:

You are given an array (which will have a length of at least 3, but could be very large) containing integers. The integers in the array are either entirely odd or entirely even except for a single integer N. Write a method that takes the array as an argument and returns N.

For example:

[2, 4, 0, 100, 4, 11, 2602, 36]

Should return: 11

[160, 3, 1719, 19, 11, 13, -21]

Should return: 160

Any comments on the following code appreciated. (I used Python 2.7)

def ifeven(list):
     #Determine if we are dealing with list of evens or odds
    sum = 0
    for ran in (0,1,2):
        sum += abs(list[ran])%2
    avg = float(sum)/3
    r_avg = round(avg)
    return r_avg == 0

def find_outlier(integers):
    even =  ifeven(integers)
    new = []
    for num in integers:
        new.append(num%2)
    if even:
        loc = new.index(1)
    else:
        loc = new.index(0)
    return integers[loc]
\$\endgroup\$

3 Answers 3

1
\$\begingroup\$

Your code is alright, but you should follow the following:

  1. Don't overwrite builtins, sum, list.
  2. Use sum.
  3. Use comprehensions.
  4. Use ternary operators.

If you use all the above, you code can become much more concise. And if you leave small one-liners on a few lines can make quite a nice program.

def ifeven(list_): # 1
    total = sum(list_[i] % 2 for i in (0, 1, 2)) # 1, 2, 3
    r_avg = round(total / 3.0)
    return r_avg == 0

def find_outlier(integers):
    even =  ifeven(integers)
    new = [i % 2 for i in integers] # 3
    loc = new.index(1 if even else 0) # 4
    return integers[loc]

If I were to write this program from scratch, then I'd do ifeven and find_outlier in the same function. This is as then you can use the comprehension i % 2 for i in ... once. After this you just need to obtain the outliers parity. Where the logic is pretty much if the first parity is either the second or thirds, then the parity you want to find is opposite to the firsts, otherwise the firsts. Or more concisely (the first parity is either the second or thirds) XOR the firsts parity. And so, I'd use:

def find_outlier(nums):
    parities = [n & 1 for n in nums]
    return nums[parities.index((parities[0] in parities[1:3]) ^ parities[0])]
\$\endgroup\$
0
3
\$\begingroup\$

The code in the post, and the code in the answers by Joe Wallis and 200_success, traverses the whole of the input to collect the parities of all the items. But it is easy to return the result as soon as the outlier has been identified, avoiding a wasted traversal of the remainder of the input.

def outlier(it):
    """Given an iterable with at least 3 items, all of which have the
    same parity except for a single outlier, return the outlier.

    """
    count, item = [0, 0], [None, None]
    for n in it:
        parity = n % 2
        if count[parity] == 0 and count[1 - parity] >= 2:
            return n
        elif count[1 - parity] == 1 and count[parity] >= 1:
            return item[1 - parity]
        count[parity] += 1
        item[parity] = n
    else:
        raise ValueError("bad iterable")
\$\endgroup\$
2
  • \$\begingroup\$ I was just going to raise two more issues, and you beat me to raising the efficiency issue. A bonus of your function that you didn't point out is that it works with any sequence, whereas the original code accepts only subscriptable lists (as required by the problem statement), but would fail if it were fed a generator expression. \$\endgroup\$ Jun 15, 2016 at 8:57
  • \$\begingroup\$ Damn! what was your thinking process behind the above solution? \$\endgroup\$
    – vivek
    Dec 4, 2016 at 12:25
2
\$\begingroup\$

Your code could be less verbose. One simplification is to replace both of your for loops with more idiomatic expressions, as shown below.


The comment for ifeven() should be a docstring, and you could clarify what you mean by "we are dealing with".

All of that stuff with avg and r_avg is a roundabout way of checking whether the sum (which could be 0, 1, 2, or 3) is either 0 or 1.

def ifeven(list):
    """Determine if the first three items are mostly even numbers."""
    return sum(n % 2 for n in list[:3]) <= 1

new is a poor variable name; I suggest parities instead. As I mentioned, it should be defined as a list comprehension.

When setting loc, you can take advantage of the fact that False and True can be promoted as integers.

def find_outlier(integers):
    parities = [num % 2 for num in integers]
    loc = parities.index(ifeven(integers))
    return integers[loc]

Observing that both ifeven() and find_outlier() perform % 2, I suggest a different way to decompose the work.

def majority(a, b, c):
    return b if a == b else c

def find_outlier(integers):
    parities = [num % 2 for num in integers]
    loc = parities.index(not majority(*parities[:3]))
    return integers[loc]
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.