#25225 closed Cleanup/optimization (fixed)
Unnecessary/Redundant __iter__ method with ListMixin class (contrib.gis.geos.mutable_list.py)?
Reported by: | Joshua Bixby | Owned by: | nobody |
---|---|---|---|
Component: | GIS | Version: | 1.8 |
Severity: | Normal | Keywords: | __iter__ geos gis ListMixin |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The ListMixin
class (contrib.gis.geos.mutable_list.py
) currently has an __iter__
method and __getitem__
method. Typically iteration is implemented using one or the other, not both. Looking at the two methods, the __iter__
method appears to be unnecessary since the Python interpreter will use __getitem__
to iterate if __iter__
is not present. Also, stepping through code appears to have __iter__
call __getitem__
, which raises the question of why define an __iter__
method when the interpreter can use __getitem__
directly.
Currently, __getitem__
can't be used properly for iteration because of the custom IndexError
raised in the _checkindex
method. Switching from raising a django.contrib.gis.geos.error.GEOSIndexError
exception to a exceptions.IndexError
will allow for the interpreter to use __getitem__
for iteration, which would allow removing the unnecessary or redundant __iter__
method.
Change History (6)
comment:1 by , 9 years ago
Description: | modified (diff) |
---|
comment:2 by , 9 years ago
Easy pickings: | unset |
---|
comment:3 by , 9 years ago
It looks like #4740 is the reason the way this code is the way it is, but the template engine lookups now catch IndexError
too, so we may be able to remove the custom exception.
Could you propose a patch with a regression test?