0
i = 0

def find_max(seq):
if i == len(seq) - 1:
      return seq[0]
else:
      first = seq[i]
      i = i + 1
      max_of_rest = find_max(seq)
      return max(first, max_of_rest)

I do not know what is wrong with this function? It is a infinite loop.

Dhara Parmar
  • 7,579
  • 1
  • 11
  • 25
Jayyyyyy
  • 215
  • 1
  • 6
  • Why would one do this recursively? you create lots of calls stackframes onto each other to use buildin `max` multiple time whereas `max(seq)` would do it in linear time and fewer memory needed? – Patrick Artner Nov 06 '19 at 06:49
  • @Patrick, one would do this recursively to learn programming. Why would someone starting to learn would need advice about stackframes and big O and memory usage with such a simple question? – MikeMajara Nov 06 '19 at 06:50
  • @Mike there are "good" problems and "bad" problems for recursive programming - trees and linked lists come to mind. Doing recursion on a linear problem just for the sake of doing recursion is simply bad. – Patrick Artner Nov 06 '19 at 08:12
  • @Patrick then I find doing a constructive comment, or better even, posting an answer as the one you did, a better approach, as opposed to questioning a post for the sake of questioning. – MikeMajara Nov 06 '19 at 09:47

5 Answers5

1

Please check out the following solution and follow comments:

def find_biggest(_list, max_element, first_run):
    """
    _list is a list of floats or integers or both,
    max element is used to store max value,
    first run checks if _list is not empty
    """
    if first_run and not _list:  # check if _list is not empty
        raise ValueError("_list should have float or integer values inside")

    first_run = False

    if not _list:  # exit from recursion, we checked all elements
        return max_element

    element = _list.pop()  # take one element

    if type(element) not in (int, float,):  # check element type
        raise TypeError("_list should contain only int or float values")

    if element >= max_element:  # check if it is more than max
        max_element = element

    return find_biggest(_list, max_element, first_run)  # next step of recursion


if __name__ == "__main__":
    # test
    print(find_biggest([-1, 4, 2, 3, 1, 0, 10, 3, 1, 7], 0, True))
    # print(find_biggest([], 0, True))  # empty case
    # print(find_biggest([-1, 4, 2, 3, "1", 0, 10, 3, 1, 7], 0, True))  # string in list
Artiom Kozyrev
  • 2,083
  • 1
  • 8
  • 20
0

You can check this:

def find_max(seq):
    if len(seq) == 1:
        return seq[0]
    else:
        if seq[0] > seq[1]:
            seq.pop(1)
        else:
            seq.pop(0)
        return find_max(seq)

Your code has a lot of indentation issues, that may skip execution of some lines. Your code should look like this:

i = 0

def find_max(seq):
    global i
    if i == len(seq) - 1:
        return seq[0]
    else:
        first = seq[i]
        i = i + 1
        max_of_rest = find_max(seq)
        return max(first, max_of_rest)

You missed the global, and thus there is no definition of i inside the function.

Cyttorak
  • 12,969
  • 3
  • 16
  • 38
0

You are using uneeded i variable, in recursion you have base case (your first if), and recursion case, which in this case would be accessing first and second element of your list. As you already checked that the list seq has more than 1 element, you can confidently access positions 0 and 1 of the list.

In your specific case, you are not really using recursion because you never reduce your case, but instead you increment an i variable, whilst recursion is based on always calling the same function with a "simpler" or reduced problem.

With that in mind, several things can be improved in your solution.

i = 0 # Not adviced

def find_max(seq):
    # Here you want to check length, making it
    # depend on i = problems
    if i == len(seq) - 1: 
        return seq[0]
    else:
        first = seq[i] # Remove all references to i
        i = i + 1 # Remove
        # Here you want to call the function with the list
        # except the element you know is lower, making the problem
        # smaller until you get to the base case
        # Instead you are calling find_max with the same
        # sequence you had (infinite loop) and returning a
        # totally different result.
        max_of_rest = find_max(seq)
        return max(first, max_of_rest)

A complete solution based on your code would look like this

def find_max(seq):
    if len(seq) == 0:
        return None
    if len(seq) <= 1:
        return seq[0]
    else:
        current_max = max(seq[0], seq[1])
        reduced_seq = [current_max] + seq[2:]
        return find_max(reduced_seq)
MikeMajara
  • 661
  • 6
  • 16
  • Yours will crash (IndexError) for: `find_max([])` and builds intermediate inner lists that are not needed for the recursion, but works otherwise – Patrick Artner Nov 06 '19 at 08:26
0

Your code contains an IndentationError and does not reduce its data on recursive calls - hence data never getting shorter - hence never ending recursion:

def find_max(seq):
    if i == len(seq) - 1:   # fixed indentation here and below
          return seq[0]
    else:
          first = seq[i]
          i = i + 1
          max_of_rest = find_max(seq)  # never reducing your data size, hence endless
          return max(first, max_of_rest)

This would be a fixed recursive solution:

def find_max(seq):   
    if not seq:
        return None  # or raise ValueError if you want find_max([]) to crash
    if len(seq) == 1:
        return seq[0]
    else:
        return max(seq[0], find_max(seq[1:]))

The problem is inheritently bad for recursive solutions, it is far better to solve it linearly (no max(..) calls needed):

def find_max_lin(seq):
    if not seq:
        return None
    m = seq[0]
    for value in seq[1:]:
        m = m if value < m else value
    return m

or even better simply use the built in max(sequence):

def find_max_builtin(seq): 
    # no need to create your own function for that though
    return max(seq)

See ternary operator for an explanation of what m = m if value < m else value does.

Patrick Artner
  • 43,256
  • 8
  • 36
  • 57
-2
i = 0

def find_max(seq):
    global i
    if i == len(seq) :
        return seq[0]
    else:
          first = seq[i]
          i = i + 1
          max_of_rest = find_max(seq)
          return max(first, max_of_rest)



print(find_max([-10,2,4,-5]))

thank me later

Karandeepdps
  • 174
  • 10