Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add callback function register on construct #48

Closed
wants to merge 1 commit into from

Conversation

joe1234wu
Copy link

This feature can able let caller do the callback function when the data going to pop

@tkem
Copy link
Owner

tkem commented Sep 1, 2015

Why would you do that? Why not override popitem() directly?

@joe1234wu
Copy link
Author

Since If I need to override popitem(), then I need to Inherit from one of the Cache.
For example, most of all I am using LRUCache and TTLCache, but I have different handling when using each of Cache.
Or more complex, I have five different LRUCache but want different popitem handing.
In this situation, you suggest that I need to Inherit and define five different Class to fulfill.

But with the callback function, I can just define five callback function and use the same LRUCache.
For me, maybe it will be more clean in implementation.

@tkem
Copy link
Owner

tkem commented Sep 1, 2015

You could implement a class decorator that wraps a cache class with a custom popitem method; for example (quickly hacked together, not something I recommend for production use):

def CallbackCache(callback):
    def decorator(cls):
        class Wrapper(cls):
            def popitem(self):
                key, value = cls.popitem(self)
                callback(key, value)
                return key, value
        return Wrapper
    return decorator

def printpop(key, value):
    print('pop item (%r, %r)' % (key, value))

@CallbackCache(printpop)
class PrintCache(cachetools.LRUCache):
    pass

# or, if you prefer

PrintCache = CallbackCache(printpop)(cachetools.LRUCache)

cache = PrintCache(maxsize=3)

Frankly, I don't plan to add more arguments to the cache constructor. I'd actually like to get rid of the missing and getsizeof arguments, if I can provide a sensible alternative, possibly using decorators. However, this isn't high on my agenda and won't happen too soon.

@tkem tkem added the wontfix label Sep 15, 2015
@tkem
Copy link
Owner

tkem commented Sep 15, 2015

No need to add this functionality to the library - can already be implemented by clients without much effort.

@tkem tkem closed this Sep 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants