Opened 15 years ago
Closed 12 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 , 15 years ago
| Attachment: | simplified_remove.diff added |
|---|
comment:1 by , 15 years ago
| Component: | Uncategorized → Core framework |
|---|
follow-up: 3 comment:2 by , 15 years ago
| Needs tests: | set |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:3 by , 15 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 , 15 years ago
| Owner: | changed from to |
|---|
comment:5 by , 15 years ago
| Cc: | added |
|---|
comment:6 by , 15 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 , 12 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.