DEV Community πŸ‘©β€πŸ’»πŸ‘¨β€πŸ’»

Sergey Kislyakov
Sergey Kislyakov

Posted on

Be careful with .pop

I've been fighting with a weird bug in my app: if I'm sending a push notification to 2+ users, only the first one receives the notification.

The bug was that I .poped the message key from the data.

Here's how it works (pseudo-code):

def send(users: [User], data: dict):
    notifications = reduce(lambda acc, user: acc + [Notification(user, data)], users, [])
    db.bulk_insert(notifications)
    for notification in notifications:
        notification.send()

...

def Notification.send(self):
    message = self.data.pop('message') 
    Firebase.send(message, self.data)
Enter fullscreen mode Exit fullscreen mode

The first notification will be sent and I thought that message would be popped out of self.data, but actually it would be popped out of the dict that self.data points to! That means that every Notification object that has been initialized in the send function using Notification(user, data) (and not Notification(user, data.copy())) would not have data['message'] anymore, leading to unexpected behavior.

If you really need to get rid of a key in a dict, you should .copy() it before mutating.
If you don't - just stick to .get().

Top comments (5)

Collapse
 
rhymes profile image
rhymes

Good catch Defman!

Keep in mind that copy() does not create a deep copy so if you modify the Notification objects... both references will point to the same Notification object that you modified :D

If you have memory constraints and you want to let the GC do its work you could investigate the usage of weakref:

A primary use for weak references is to implement caches or mappings holding large objects, where it’s desired that a large object not be kept alive solely because it appears in a cache or mapping.

Collapse
 
defman profile image
Sergey Kislyakov • Edited on

Keep in mind that copy() does not create a deep copy so if you modify the Notification objects... both references will point to the same Notification object that you modified :D

I don't quite understand that. Doesn't {"a": 1, "b": {"c": 2}}.copy() create a copy of a dict (and allocate it in the memory) that would pass assert id(Notification1.data) != id(Notification2.data) (so by modifying Notification1.data I would not affect Notification2.data)?

Collapse
 
rhymes profile image
rhymes • Edited on

copy() performs a shallow copy:

In [1]: inner_dict = {"c": 2}

In [2]: d = {"a": 1, "b": inner_dict}

In [3]: c_d = d.copy()

In [4]: inner_dict["x"] = 3

In [5]: inner_dict
Out[5]: {'c': 2, 'x': 3}

In [6]: d
Out[6]: {'a': 1, 'b': {'c': 2, 'x': 3}}

In [7]: c_d
Out[7]: {'a': 1, 'b': {'c': 2, 'x': 3}}

In [8]: id(d['b']), id(c_d['b'])
Out[8]: (4364454576, 4364454576)

as you can see both d and its copy c_d point to the same inner_dict which when modified it reflects on both the original and the copy.

TLDR; if any of the values of the dict that's about to be copied is a reference to an object (in this case another dictionary, in your case a Notification object) then the original and the copy will point to the same object.

Thread Thread
 
defman profile image
Sergey Kislyakov

I see. Thanks!

In my case, data does not hold a Notification object. It doesn't hold any references at all, actually. It's a simple dict that looks like this: {"message": "my message", "some_attr": 123}.

Collapse
 
itr13 profile image
Mikael Klages • Edited on

If you're working with reference-types, it's probably better to never use mutating functions unless you absolutely have to.

DEV

Thank you.

Β 
Thanks for visiting DEV, we’ve worked really hard to cultivate this great community and would love to have you join us. If you’d like to create an account, you can sign up here.