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]
3 Answers 3
Your code is alright, but you should follow the following:
- Don't overwrite builtins,
sum
,list
. - Use
sum
. - Use comprehensions.
- 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])]
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")
-
\$\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\$200_success– 200_success2016年06月15日 08:57:29 +00:00Commented Jun 15, 2016 at 8:57
-
\$\begingroup\$ Damn! what was your thinking process behind the above solution? \$\endgroup\$vivek– vivek2016年12月04日 12:25:39 +00:00Commented Dec 4, 2016 at 12:25
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]
Explore related questions
See similar questions with these tags.