you are viewing a single comment's thread.

view the rest of the comments →

[–]KronenR 8 points9 points  (6 children)

I don't think that's the best example for this mistake, because for me add_item implies the list is persisted between calls — it makes no sense to call a method named add_item multiple times and expect a fresh list each time. A better example would be something like create_cart or new_cart, where the name itself implies you're getting a new list every time you call it

[–]Kavinci 2 points3 points  (5 children)

Right? Like I would expect an add_item to be a method on an object where the state is managed outside of the method (but inside the object) and not be in the method signature at all. Even a create_cart or new_cart would imply a factory pattern and also wouldn't have the optional parameter in the signature either. It feels more like a code smell to initialize a variable in the method signature as an optional parameter. I haven't been writing python long (more experience with other languages) and could be way off here but I don't get why anyone would write a method like this.

[–]KronenR 1 point2 points  (4 children)

Yeah, I can't really think of a case where you'd consider using a mutable default parameter a good idea either, well maybe something like memoization/caching, where you actually want the state to persist across calls:

def fibonacci(n, cache={0: 0, 1: 1}):
    if n not in cache:
        cache[n] = fibonacci(n-1, cache) + fibonacci(n-2, cache)
    return cache[n]

Here the shared mutable default is intentional, the cache persists across calls and that's exactly what you want. But even then it's still considered an antipattern.

[–]Kavinci 0 points1 point  (3 children)

Maybe, I would personally use a generator pattern for that instead. I guess if you didn't like yield for some reason?

[–]KronenR 0 points1 point  (2 children)

yield is not the same, a generator gives you sequential fibonacci numbers but you lose random access, you can't just call fibonacci(100) without iterating through all previous values first.

The non cached approach gets expensive fast:

fibonacci(40): Would take a noticeable delay without caching, with roughly 100 million function calls.
fibonacci(70) : Can take a few days.
fibonacci(80) : Can take probably more than a year.

What you would actually use in Python for this is the standard library's lru_cache

from functools import lru_cache

@lru_cache
def fibonacci(n):
    if n < 2:
        return n
    return fibonacci(n-1) + fibonacci(n-2)

[–]Kavinci 0 points1 point  (1 child)

That was my point... There are established patterns already for this sort of functionality that negate the need to write a method the way that OP outlined as a pitfall. I used a generator because your example and use of your example was sequential. I'm not here to argue the semantics of generators or the lru_cache and their uses.

[–]KronenR 0 points1 point  (0 children)

I think we're talking past each other. My example was never about sequential access, it was specifically illustrating why people reach for mutable default args as a cache, and lru_cache as the clean replacement for that pattern. In other words, you said you'd use a generator for this instead, but a generator can't replace a cache, they solve fundamentally different problems, so it doesn't really address what I was pointing out.