Opened 18 years ago
Closed 18 years ago
#4041 closed (fixed)
cache backends should implement __contains__ so that the in operator may be used
Reported by: | Owned by: | Jacob | |
---|---|---|---|
Component: | Core (Cache system) | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
So instead of
cache.has_key(mykey)
one could do
mykey in cache
Attachments (2)
Change History (10)
by , 18 years ago
comment:1 by , 18 years ago
Component: | Uncategorized → Cache system |
---|---|
Has patch: | set |
Needs documentation: | set |
Triage Stage: | Unreviewed → Design decision needed |
comment:2 by , 18 years ago
Triage Stage: | Design decision needed → Ready for checkin |
---|
comment:3 by , 18 years ago
So in my patch, the __contains__
method in BaseCache
simply calls has_key
. This is nice since all the cache subclasses automatically pick this up for free, but also means that a
mykey in cache
statement becomes slower than
cache.has_key(mykey)
because of the extra function call. If we care about performance here, we might want to implement __contains__
for each cache class without calling that class's has_key
method. Looking at the python standard library, it seems that classes with both __contains__
and has_key
methods just put the same code in each instead of having one call the other.
follow-up: 6 comment:4 by , 18 years ago
Needs documentation: | unset |
---|---|
Patch needs improvement: | set |
Triage Stage: | Ready for checkin → Accepted |
Not yet ready to checkin, due to Gary's last comment. Checking in an optimisation that slows the code down seems like something we should avoid.
Note sure why this was marked as needing documentation, either, but if you think it needs documentation, Gary, by all means write some.
follow-up: 7 comment:5 by , 18 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Unless I'm missing something, can't we just do this? (see patch)
comment:6 by , 18 years ago
Replying to mtredinnick:
Note sure why this was marked as needing documentation, either, but if you think it needs documentation, Gary, by all means write some.
I was just thinking it might be good to be mentioned in the cache documentation (there is no mention of has_key()
method currently).
comment:7 by , 18 years ago
Replying to SmileyChris:
Unless I'm missing something, can't we just do this? (see patch)
Yes, it is possible to make __contains__
and has_key
be aliases for each other like this. In the early days, when __contains__
was first introduced into the language, it wasn't clear if this was going to be a good identification, but nowadays, it's accepted as correct; to the point that even Python core checkins are using this idiom in places.
What that patch doesn't need is the comment: it's the equivalent of saying "add one to i" in a comment. People know what __contains__
does already.
comment:8 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
added
__contains__
toBaseCache
class