Opened 14 years ago
Closed 11 years ago
#13734 closed Cleanup/optimization (fixed)
Simpler implementation of django.dispatch.dispatcher.Signal._remove_receiver
Reported by: | Satoru Logic | Owned by: | Satoru Logic |
---|---|---|---|
Component: | Core (Other) | Version: | 1.2 |
Severity: | Normal | Keywords: | dispatcher, signal |
Cc: | Russell Keith-Magee, satorulogic@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
In Django 1.2.1, the _remove_receiver method of the django.dispatch.dispatcher.Signal class is implemented as:
def _remove_receiver(self, receiver): """ Remove dead receivers from connections. """ to_remove = [] for key, connected_receiver in self.receivers: if connected_receiver == receiver: to_remove.append(key) for key in to_remove: for idx, (r_key, _) in enumerate(self.receivers): if r_key == key: del self.receivers[idx]
I think this can be simplified by iterating the receivers list reversely and removing the specified receiver by index:
def _remove_receiver(self, receiver): """ Remove dead receivers from connections. """ for idx in xrange(len(self.receivers)-1, -1, -1): (_, connected_receiver) = self.receivers[idx] if connected_receiver == receiver: del self.receivers[idx]
Attachments (1)
Change History (10)
by , 14 years ago
Attachment: | simplified_remove.diff added |
---|
comment:1 by , 14 years ago
Component: | Uncategorized → Core framework |
---|
follow-up: 3 comment:2 by , 14 years ago
Needs tests: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 14 years ago
Cc: | added |
---|---|
Needs tests: | unset |
Replying to russellm:
Marking as 'needs tests' because I'm not completely certain of the extent to which this disconnection behavior is tested at present; if it isn't, an optimizatin like this should is a reasonable excuse to beef up the tests.
If it turns out I'm mistaken and this *is* tested, feel free to leave a note to that effect and drop the flag.
This is not a disconnection
behavior. Rather, _remove_receiver
is registered as a weakref
callback when the receiver
object is garbage collected.
I think this behavior is tested here http://code.djangoproject.com/browser/django/trunk/tests/regressiontests/dispatch/tests/test_dispatcher.py#L55.
comment:4 by , 14 years ago
Owner: | changed from | to
---|
comment:5 by , 14 years ago
Cc: | added |
---|
comment:6 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Cleanup/optimization |
comment:7 by , 14 years ago
Easy pickings: | unset |
---|---|
Patch needs improvement: | set |
simplified_remove.diff fails to apply cleanly on to trunk
comment:9 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
That part of the code has changed a lot recently, in particular with 52cad43bc3d3126fcf7e08582373c12e88895cd3 and c29d6f767691cceb9964c0d212e01281ac6721d3
This report is now obsolete.
Marking as 'needs tests' because I'm not completely certain of the extent to which this disconnection behavior is tested at present; if it isn't, an optimizatin like this should is a reasonable excuse to beef up the tests.
If it turns out I'm mistaken and this *is* tested, feel free to leave a note to that effect and drop the flag.