This is my code. Is there anything that can be improved?

https://www.codecademy.com/courses/learn-python-3/lessons/python-functions-loops-cc/exercises/max-num?action=resume_content_item

#Write your function here
def max_num(nums):
  while len(nums) > 1:
    if nums[0] >= nums[-1]:
      nums.pop()
    else:
      nums.pop(0)
  return nums[0]

#Uncomment the line below when your function is done
print(max_num([50, -10, 0, 75, 20]))
1 Like

It probably does too much.
We don’t need to pop anything.

def max_num(nums):
  return max(nums)
print max_num([1,2,3])

That would be a more pythonic way of doing things
If you want to do the iterating and looping yourself you still don’t have to pop things

2 Likes

Removing elements from the front of a list involves moving all the following elements one step to cover the empty gap.
You’re better off removing from the back.
Except that there’s no need to remove after you’ve looked at an element. You just need to look at them.
There’s a bug too. What’s the maximum of [1]?

2 Likes

Was trying to find a way to discourage pop since it is so destructive. A function should not destroy a data structure unless that is the intention. In this case, we might want to preserve the data structure and only find the maximum value.

>>> def max_num(nums):
    for i in range(1, len(nums)):
        if nums[i-1] > nums[i]:
            nums[i], nums[i-1] = nums[i-1], nums[i]
    return nums[-1]

>>> max_num([89,46,98,64,39,99,80])
99
>>> 

The above sort of destroys the original list, but all the data is still present, if a little bit jumbled from the original.

We can do this without swapping data points, though.

>>> def max_num(nums):
    m = nums[0]
    for i in range(1, len(nums)):
        if nums[i] > m:
            m = nums[i]
    return m

>>> max_num([89,46,98,64,39,99,80])
99
>>> 

That one does nothing to the list.

4 Likes

That’s exactly the behaviour of reduce, starting with the first value and then doing something between it and each of the rest

def max_num(nums):
    return reduce(lambda a, b: a if a > b else b, nums)

# or:
def max_num(nums):
    return reduce(max, nums)
# (this is arguably different from:
def max_num(nums):
   return max(nums)
# because it uses a simpler use-case of max: only two values,
# as opposed to a whole list.
def max(a, b):
    return a if a > b else b

So a nice way to solve the problem would be to implement both reduce and max, and then combine them to get maximum:

def reduce(f, iterable, acc=None):
    iterator = iter(iterable)
    if acc is None:
        acc = next(iterator)
    for e in iterator:
        acc = f(acc, e)
    return acc

and then:

from functools import partial

max_num = partial(reduce, max)

…i guess someone will now think I should be implementing partial too.

def partial(f, arg1):
    def wrapper(*args):
        return f(arg1, *args)
    return wrapper

but with those tools in place one can also implement lots of other things. like sum and product:

def add(a, b):
    return a + b


def mul(a, b):
    return a * b


sum = partial(reduce, add)
product = partial(reduce, mul)
3 Likes

I think I’m still a bit silly about subscripts and indexing. And I can’t use them flexibly. Every time I need to use subscripts and indexes, my brain will habitually start thinking from 0 and [0].The solution to the problem.Can’t be used flexibly like this`

`
Starting from 1 or [1].
The translation software seems to be a bit problematic, I hope you can understand it.

2 Likes

range arguments can only be integer, so yes, starting from 1. len is only able to return integers so that part of range is status quo.

2 Likes

If looking at each value you’d start from 0

But what’s going on here is looking between two values

a
   <- compare here
b
   <- compare here
c
   <- compare here
d
   <- compare here
e

That is a bit trickier.

2 Likes