5

Well, wondering what is the suggested way (i.e. best practice) to pass some dictionary to function/method, modify it and "return" to the user?

For example, as dictionary is mutable, we can simply do:

my_dict = {
    'a': 'b'
}

def foo(bar):
    # Do something with `bar`...
    ...
    bar['c'] = d  # Update `bar`.

foo(my_dict)
# Do something with modified `my_dict`...

This is what I was doing mostly in the past.

But another, more explicit, way to get the same effect would be:

my_dict = {
    'a': 'b'
}

def foo(bar):
    # Do something with `bar`...
    ...
    bar['c'] = 'd'  # Update `bar`.
    return bar

my_dict = foo(my_dict)
# Do something with modified `my_dict`...

This maybe follows better one of the most important Python principles: "Explicit is better than implicit.", but definitely requires little more writing (maybe unnecessarily).

Of course, there are also other approaches to achieve the same, but wondering which is preferred way between above two?

P.S. This is my first question ever on StackOverflow (for 10+ years in programming)... I tried to search for similar use-cases (as always), but incredible that nobody ever asked.

EDIT: Updated examples to be more specific.

Wolf
  • 98
  • 7
  • 3
    Generally, it is considered *bad practice* for a function to mutate its inputs to begin with. But given that, it is actually returning `None` that is considered idiomatic and more explicit. When you return the object, it is unclear if it is a new object or the same object that was passed in. In Python, it would be assumed that it is not the same object, and would be surprising if the object being passed in were mutated. – juanpa.arrivillaga Jul 06 '20 at 22:45
  • @juanpa.arrivillaga Are you saying that's perfectly fine to use approach from the first example, i.e. without explicitly returning mutated dict object? That was my preferred way in such situations, but would like to hear people opinions... – Wolf Jul 07 '20 at 22:11
  • 1
    I would say that *mutating your inputs is not perfectly fine*, it's a well-known antipattern, but if you are *going to do that regardless* then idiomatically the function should return `None`. That would be considered *more explicit* because returning an object makes it ambiguous if that is the behavior or not. I'm not sure why you consider returning the `dict` more explicit. – juanpa.arrivillaga Jul 07 '20 at 22:14
  • 1
    Nobody ever asked? [Relevant discussion from five years ago](https://stackoverflow.com/questions/26027694/correct-style-for-python-functions-that-mutate-the-argument). [Another relevant discussion with a Java context](https://softwareengineering.stackexchange.com/questions/245767/is-modifying-an-incoming-parameter-an-antipattern). – Dennis Sparrow Jul 07 '20 at 22:33
  • @juanpa.arrivillaga Got it, and makes sense. However, if that's well-known antipattern (or _bad practice_ by some other opinions), what would be proper way to achieve the same in Python? Of course, we can always create new instance based on the original one, modify it as we want and return that one as a result (like already proposed in one of the answers)... But here we could possible have other penalties (performance, memory) when manipulating with large objects. – Wolf Jul 08 '20 at 11:36
  • @DennisSparrow Indeed, it's already answered (practically the same opinions like here). Somehow I was not able to find that answer. Appreciate, thanks! – Wolf Jul 08 '20 at 11:39

3 Answers3

4

I'd say better to not return

Well look no further than list.sort()

It sorts list in-place, meaning original list changes, and nothing is returned.

The reason for returning None is so users do not get confused (thinking original list is preserved)

The most important thing is to properly name what you're doing
e.g.

def fill_with_defaults(cfg):
    ...

And then fill_with_defaults(configuration) is looking perfectly readable

And as for "explicit", no return value is even better because the reader knows something is changed in-place
meaning he wouldn't consider config2 = fill_with_defaults(configuration)
leading to buggy code (cuz changing one config affects other)

Superior
  • 787
  • 3
  • 17
1

If you're not using the keys or values in bar why is it necessary to pass it in:

my_dict = {
    'a': 'b'
}

def foo():
    return dict(c='d')

my_dict.update(foo())
Peter Wood
  • 23,859
  • 5
  • 60
  • 99
  • Yes, that perfectly makes sense. But thing is that I need input `bar` dict in `foo` function. I will update question... Thanks for pointing that out. – Wolf Jul 07 '20 at 21:59
1

I would do:

my_dict = {
    'a': 'b'
}

def foo(bar):
    return dict(bar, **{'c':'d'})

my_dict = foo(my_dict)

It's essentially merge bar with whatever the other dict is, taking the latest key-value in case of conflict.

Not sure if it's canonical or most pythonish, however it's pretty clean way of doing what you're trying to do...

Grzegorz Skibinski
  • 12,624
  • 2
  • 11
  • 34
  • I agree, this is pretty clean approach, but could be problematic for large dictionaries, especially when `foo` needs to be called multiple times (every call would create new dict). But when applicable, this is probably the best approach (as many other comments stated that mutating original value is considered as a _bad practice_). – Wolf Jul 08 '20 at 11:25