2

I have a function that recursively finds a solution to a maze represented by a 2D array. I declared all the default arguments in a way that would allow me to call this function from main without passing any arguments. One of these default arguments is an empty list, which I understand is bad practice. I'm wondering if this is a valid use-case of a mutable default argument, or if there's a better way to do this:

def solve_maze(self, i=0, j=0, path=[]):
    if (i, j) in self.memory:
        return False
    if self.maze[i][j] == 'D':
        return path[:-1]
    if not (directions := self.get_directions(i, j, [[i, j]] + path)):
        self.memory.append((i, j))
        return False
    move = {'l': [i, j-1], 'u': [i-1, j], 'r': [i, j+1], 'd': [i+1, j]}
    for k in range(len(directions)):
        best_dir = directions[k]
        if solution := self.solve_maze(move[best_dir][0], move[best_dir][1], [[i, j]] + path):
            return solution
    self.memory.append((i, j))
    return False

This function behaves exactly how I need it, so I don't need help making it work. Using this mutable default is not causing me issues. This is simply a personal curiosity on "proper Python coding".

I do realize the obvious solution is something akin to this:

def solve_maze(self, i=0, j=0, path=None):
    if not path:
        path = []

But this just adds clutter to my code. I'm curious what the "correct" way to do this is.

NewBoard
  • 180
  • 1
  • 9
  • 3
    basically, if you can guarantee that `path` is not being modified you should be ok. However, it is still better to avoid it to be on the safe side. Since you mentioned "proper Python coding", this is something I'd definitely flag in a code review, even if the code itself *currently* works as expected. – DeepSpace Oct 08 '21 at 00:27
  • 3
    "One of these default argument is an empty list, which I understand is bad practice" it isn't, **if you understand what is going on**. In this case, since the list is never mutated, then it is fine. – juanpa.arrivillaga Oct 08 '21 at 00:30
  • 3
    If you ask me, there is nothing wrong with this approach, as long as you understand what you're doing and the possible caveats. It is actually a very popular way of dealing with things in other languages like C/C++ -- you pass an argument by reference (pointer / address) and expect the function to alter the object. – s0mbre Oct 08 '21 at 00:31
  • 4
    @s0mbre The problem with a mutable default argument is not with the passed object being modified, but with that modification persisting **between separate, seemingly unrelated function calls** – DeepSpace Oct 08 '21 at 00:33
  • 1
    @s0mbre that's actually *not* what's happening here. I actually think that approach, in general, is bad (although in languages like C/C++, it is a compromise you make for the sake of performance) – juanpa.arrivillaga Oct 08 '21 at 00:34
  • 1
    Honestly, the recursion here is irrelevant – juanpa.arrivillaga Oct 08 '21 at 00:43
  • Was forced to use recursion by the project description. – NewBoard Oct 08 '21 at 00:44
  • 1
    As long as this 'byref' argument (path) is not returned or passed into the function from outside, the code looks OK to me. It is actually quite a well-spread practice to use a mutable in recursive functions. As it looks here, the function only returns a boolean or a slice from path. – s0mbre Oct 08 '21 at 00:49
  • 1
    @s0mbre Again, the problem is not with the argument being mutable. The problem is specifically with the argument having a mutable **default value**. See [this](https://stackoverflow.com/questions/1132941/least-astonishment-and-the-mutable-default-argument) – DeepSpace Oct 08 '21 at 00:50
  • 2
    Using mutable default arguments (and expecting the mutable argument to be reset each call) is a common error in Python code, so on first glance this might look like a mistake to other Python developers. For that reason, I would either add a comment that the mutable default argument is intended, or switch to the `path=None` style you outline in your question. It's not necessary, but doing one of these will make it easier for other developers to read your code. – Jack Taylor Oct 08 '21 at 00:50
  • 1
    @NewBoard You might be interested in [this](https://stackoverflow.com/questions/9158294/good-uses-for-mutable-function-argument-default-values) question – DeepSpace Oct 08 '21 at 00:56
  • 1
    You don't really want to share the value of `path` with *every* call to `solve_maze`, only the recursive calls from a single "top-level" call. Share the value would be fine if you only intended to make a single top-level call in your program, but it's better to define the function more generally. – chepner Oct 08 '21 at 01:16
  • 1
    Nothing wrong with using default arguments for _every_ parameter, if the function calls for it. I wrote a python maze solver in [this Q&A](https://stackoverflow.com/a/65512563/633183) that I think you will find helpful :D – Mulan Oct 08 '21 at 03:47

1 Answers1

1

I'd say the problem isn't that you're using a mutable object there, but that you're not using it... as such. You grow your path with [[i, j]] + path. To build a path of length n, that takes Θ(n²) time and space. Would be more efficient to actually modify the list, i.e., append to and pop from it. Preferably at the end instead of the start like you do now. If you do need the result in reverse, you could still reverse the complete path once you found it.

no comment
  • 6,381
  • 4
  • 12
  • 30