[epper]

First time I write in HN, but I really wanted to give my take on this.

I have to say that I really feel that to get readability, over nesting too, you often should refactor a bit the code.

I would in fact write the get_cached_user method by using separate methods and a @cache decorator. Every single function is very readable by itself.

(I'm not fluent in python, pseudo-python follows... but you should get the idea)

    def cache(function_to_cache):
        '''
        A Decorator that caches a function result
        '''
        def wrapper(param):
            cache_key = function_to_cache.name + " on " + param
            if cache.contains(cache_key):
                return cache.get(cache_key)
            else
                value = function_to_cache(param)
                cache.set(cache_key, value)
                return value
        return wrapper
    
    @cache
    def get_cached_user_by_id(id):
        return db.get_user_by_id(id)
    
    @cache
    def get_cached_user_by_username(username):
        return db.get_user_by_username(username)
        
    
    def get_cached_user(user_id = None, username = None):
        user = None
        if user_id:
            user = get_cached_user_by_id(user_id)
        else if username:
            user = get_cached_user_by_username(username)
        
        if not user:
            raise ValueError('User not found')
        
        return user

[kansface]

I don't think I'd want to maintain this bit of code although I'd prefer it to the mess of nesting in the original implementation. Each separate way of caching gets its own function which is ... ok I guess. Its explicit, which is pythonic, but overly verbose for multiple methods and we have to draw the line somewhere.

I think decorators are complex enough that I can't immediately look at the thing and know what its doing in the same way as early returns. A closure that takes a function pointer with some introspection magic is just a lot of mental juggling. At least with your case the complexity doesn't compound and is pushed as low as possible (as per Code Complete).

A final note: functions can have static members just like classes in python. fuction_to_cache.name would access the .name member of the function if such a thing existed (it will be an AttributeError? in this case). You are looking for function.__name__ I believe.

[epper]

I don't think that decorators are too complex to be used usually... In python it should be a known and understood pattern and therefore it should not scare at all.

(Moreover it should handle multiple args, and there are better and more popular implementation such as @memoize)

However my assumption in this specific case is that the project is not small and therefore that decorator and this "pattern" could be used somewhere else too. If this is not the case I agree with you that it would just add too much complexity and it would hence be better to not have the decorator at all. The get_cached_user_by_username and get_cached_user_by_id would just handle the cache hit/miss by themselves.

Still it would be much more readable, because the main point I would say is to separate a long or too nested method in more methods.

    def get_cached_user_by_id(id):
        user = cache.get_user_by_id(id)
        if not user:
            user = db.get_user_by_id(id)
            cache.set_user(user)
        return user
    
    def get_cached_user_by_username(username):
        user = cache.get_user_by_username(username)
        if not user:
            user = db.get_user_by_username(username)
            cache.set_user(user)
        return user
    
    def get_cached_user(user_id = None, username = None):
        user = None
        if user_id:
            user = get_cached_user_by_id(user_id)
        else if username:
            user = get_cached_user_by_username(username)
        
        if not user:
            raise ValueError('User not found')
        
        return user

(Thanks for the .__name__ tip... I have not use python for a while)