Django

Code

Ticket #4041 (closed: fixed)

Opened 1 year ago

Last modified 1 year ago

cache backends should implement __contains__ so that the in operator may be used

Reported by: Gary Wilson <gary.wilson@gmail.com> Assigned to: jacob
Milestone: Component: Cache system
Version: SVN Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

So instead of 

cache.has_key(mykey)

one could do 

mykey in cache

Attachments

4041.diff (1.0 kB) - added by Gary Wilson <gary.wilson@gmail.com> on 04/14/07 01:09:36.
added __contains__ to BaseCache class
4041.2.diff (1.0 kB) - added by SmileyChris on 04/27/07 19:57:21.
optimized version

Change History

04/14/07 01:09:36 changed by Gary Wilson <gary.wilson@gmail.com>

  • attachment 4041.diff added.

added __contains__ to BaseCache class

04/14/07 01:10:09 changed by Gary Wilson <gary.wilson@gmail.com>

  • needs_better_patch changed.
  • component changed from Uncategorized to Cache system.
  • needs_tests changed.
  • needs_docs set to 1.
  • has_patch set to 1.
  • stage changed from Unreviewed to Design decision needed.

04/14/07 02:10:53 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Design decision needed to Ready for checkin.

04/14/07 19:55:27 changed by Gary Wilson <gary.wilson@gmail.com>

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 ) 04/27/07 09:32:49 changed by mtredinnick

  • needs_better_patch set to 1.
  • stage changed from Ready for checkin to Accepted.
  • needs_docs deleted.

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.

04/27/07 19:57:21 changed by SmileyChris

  • attachment 4041.2.diff added.

optimized version

(follow-up: ↓ 7 ) 04/27/07 19:58:05 changed by SmileyChris

  • needs_better_patch deleted.
  • stage changed from Accepted to Ready for checkin.

Unless I'm missing something, can't we just do this? (see patch)

(in reply to: ↑ 4 ) 05/03/07 18:52:27 changed by Gary Wilson <gary.wilson@gmail.com>

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).

(in reply to: ↑ 5 ) 05/07/07 23:08:39 changed by mtredinnick

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.

05/07/07 23:13:46 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [5171]) Fixed #4041 -- Added a contains method to cache backends. Thanks, Gary Wilson and SmileyChris?.

Add/Change #4041 (cache backends should implement __contains__ so that the in operator may be used)




Change Properties
Action