I have a function called max_list
that returns the largest number in a randomly generated list (I know there is an inbuilt function, this is just for practice)
import random
L = []
rang = 100 #number size limit
r = 10 #size of list
for i in range(r):
L.append(random.randint(0, rang))
def max_list(L):
max_in_list = L[0]
for j in range(len(L)):
if max_in_list == rang: #if max_in_list is the highest it can be it must be the maximum
return max_in_list
elif L[j] > max_in_list: #if tere is a value greater than max_in_list it becomes the new max_in_list
max_in_list = L[j]
if j+1 == r:
return max_in_list
elif j+1 == r: #if it reaches the end of the list it must be the largest
return max_in_list
My problem comes when trying to remove the largest element in the list one by one (the idea is to also append it to a new list) so for example in the list ls = [1, 2, 3]
it should first remove the 3, then the 2 and so on
for k in range(len(L)):
L.remove(max_list(L))
The thing is, it seems to be that the list does not update and max_list(L)
remains fixed so on the second iteration it will append None
(since it was removed in the first iteration) and result in an error: L.remove(max_list(L)) ValueError: list.remove(x): x not in list
With the example I gave before (ls = [1, 2, 3]
), it would remove the 3 but on the second iteration it will try to remove the 3 instead of 2 (which would be the new max) resulting in an error
When using the inbuilt max()
function it seems to work, so I think there is something wrong with the function.
The problem is that your function does not return anything explicitly (i.e. it returns None
) when the if
conditions are never true, which can happen when your list is shorter than r
. And as you remove items, the second call of this function will indeed get a list that is shorter than r
...
The very quick fix is to append a return max_in_list
at the end of your function body.
But consider these remarks:
Instead of comparing with r
, just let the loop complete (as it is the last iteration anyway). So drop those two if
statements that test for j+1 == r
.
Instead of iterating over range(len(L))
, you can iterate over L
, which means (together with the previous remark) you don't need to work with indices: you get the values in the list.
The work done to test max_in_list == rang
may not be worth the effort, as the probability of hitting that value is rather small.
Your code assumes that the list is not empty. If you call it with an empty list you'll get an error. Maybe consider checking for the list to be empty and determine what you want to return in that case.