Problem
Write the
find_boundaries
function, which will take a list of numbers. The function should return the tuple with the smallest and largest number in the set. If there is an item in the list that is not a number, it should be ignored. If an empty list is entered, the function should returnNone
.
My code:
def find_boundaries(lista):
if not lista:
return None
elif lista:
max_ = lista[0]
min_ = lista[0]
new_lista = []
for i in lista:
if int == type(i):
new_lista.append(i)
for i in range(len(new_lista)):
if new_lista[i] > max_:
max_ = new_lista[i]
elif new_lista[i] < min_:
min_ = new_lista[i]
return max_, min_
I am looking for any opinion/ suggestion on that code. It does what is expected, but would you say that this is the most efficient way to solve that task in Python?
Solution
welcome to Code Review!
I would write your code like this:
def find_boundaries(lista):
if not lista:
return None
int_list = [x for x in lista if type(x) == int]
return (max(int_list), min(int_list))
Notice a few things:
-
If you test your parameters for validity at the beginning of the function, it’s perfectly fine not to use an
else
afterwards because thereturn
statement exits the function (short-circuiting). This saves one level of nesting and makes the function more simple. -
Python is designed to let you avoid writing loops as much as possible. Loops are a nuisance to write, take longer to read and increase cognitive load. You should get familiar with Python’s list and dict comprehensions. Notice how much clearer the code for filtering out the values that aren’t integers is.
-
Again, the
max
andmin
functions are built-in in Python and take any iterable (like a list) to return the largest and smallest element respectively. You don’t need to deal with loop indices, the length of the list, etc. You just express what it is you want to return: a tuple that contains the largest and the smallest elements.