On: Reducing Code Nesting

[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 → kansface]

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)