6

Is it good style to daisy-chain Python/Django custom decorators? And pass different arguments than received?

Many of my Django view functions start off with the exact same code:

@login_required
def myView(request, myObjectID):
    try:
        myObj = MyObject.objects.get(pk=myObjectID)
    except:
        return myErrorPage(request)       

    try:
        requester = Profile.objects.get(user=request.user)
    except:
        return myErrorPage(request)

    # Do Something interesting with requester and myObj here

FYI, this is what the corresponding entry in urls.py file looks like:

url(r'^object/(?P<myObjectID>\d+)/?$', views.myView, ),

Repeating the same code in many different view functions is not DRY at all. I would like to improve it by creating a decorator that would do this repetitive work for me and make the new view functions much cleaner and look like this:

@login_required
@my_decorator
def myView(request, requester, myObj):        
    # Do Something interesting with requester and myObj here

So here are my questions:

  1. Is this a valid thing to do? Is it good style? Notice that I will be changing the signature of the myView() function. That feels a bit strange and risky to me. But I'm not sure why
  2. If I make multiple such decorators that do some common function but each call the wrapped function with different arguments than the decorator received, is it OK if I daisy-chain them together?
  3. If it is OK to #1 and #2 above, what is the best way to indicate to the users of this myView what the set of arguments are that they should pass in (because just looking at the parameters in the function definition is no longer really valid)
Anshul Goyal
  • 73,278
  • 37
  • 149
  • 186
Saqib Ali
  • 11,931
  • 41
  • 133
  • 272
  • There's nothing technically wrong with stacking decorators, but if it's something you're going to need in a lot of views, I would recommend extending a class-based view to encapsulate the `login_required` decorator to keep your code DRY. – Brandon Taylor Mar 26 '14 at 04:02
  • But does it make worse If the decorator is not simply a pass through, but passes different arguments to the wrapped function than what it received? – Saqib Ali Mar 26 '14 at 04:53
  • Good question. Anyone with more experience care to comment? – Brandon Taylor Mar 26 '14 at 11:47

3 Answers3

6

That's a very interesting question ! Another one has already been answered in depth on the basic usage of decorators. But it does not provide much insight on modifying arguments

Stackable decorators

You can find on that other question an example of stacked decorators with the following piece of explanation hidden in a very, very long and detailed answer :

Yes, that’s all, it’s that simple. @decorator is just a shortcut to:

another_stand_alone_function = my_shiny_new_decorator(another_stand_alone_function)

And that's the magic. As python documentation states : a decorator is a function returning another function.

That means you can do :

from functools import wraps

def decorator1(f):
    @wraps(f)
    def wrapper(*args, **kwargs):
        do_something()
        f(*args, **kwargs)
    return wrapper


def decorator2(f):
    @wraps(f)
    def wrapper(*args, **kwargs):
        do_something_else()
        f(*args, **kwargs)
    return wrapper

@decorator1
@decorator2
def myfunc(n):
    print "."*n

#is equivalent to 

def myfunc(n):
    print "."*n
myfunc = decorator1(decorator2(myfunc))

Decorators are not Decorators

Python decorators might be puzzling for developpers who learned OOP with a language where GoF has already used half of the dictionary to name the patterns who fix the failures of the language is the de-facto design pattern shop.

GoF's decorators are subclasses of the component (interface) they're decorating, therefore sharing this interface with any other subclass of that component.

Python decorators are functions returning functions (or classes).

Functions all the way down

A python decorator is a function returning a function, any function.

Most decorators out there are designed to extend the decorated function without getting in the way of it's expected behavior. They are shaped after GoF's definition of the Decorator pattern, which describes a way to extend an object while keeping it's interface.

But GoF's Decorator is a pattern, whereas python's decorator is a feature.

Python decorators are functions, these functions are expected to return functions (when provided a function).

Adapters

Let's take another GoF pattern : Adapter

An adapter helps two incompatible interfaces to work together. This is the real world definition for an adapter.

[An Object] adapter contains an instance of the class it wraps. In this situation, the adapter makes calls to the instance of the wrapped object.

Take for example an object — say a dispatcher, who would call a function which takes some defined parameters, and take a function who would do the job but provided another set of parameters. Parameters for the second function can be derived from those of the first.

A function (which is a first-class object in python) who would take the parameters of the first and derive them to call the second and return a value derived from its result would be an adapter.

A function returning an adapter for the function it is passed would be an adapter factory.

Python decorators are functions returning functions. Including adapters.

def my_adapter(f):
    def wrapper(*args, **kwargs):
        newargs, newkwargs = adapt(args, kwargs)
        return f(*newargs, **newkwargs)

@my_adapter # This is the contract provider
def myfunc(*args, **kwargs):
    return something()

Oooooh, I see what you did there… is it good style ?

I'd say, hell yeah, yet another built-in pattern ! But you'd have to forget about GoF Decorators and simply remember that python decorators are functions which return functions. Therefore, the interface you're dealing with is the one of the wrapper function, not the decorated one.

Once you decorate a function, the decorator defines the contract, either telling it's keeping the interface of the decorated function or abstracting it away. You don't call that decorated function anymore, it's even tricky to try it, you call the wrapper.

Community
  • 1
  • 1
ddelemeny
  • 1,891
  • 13
  • 18
5

First of all, this block of code:

try:
    myObj = MyObject.objects.get(pk=myObjectID)
except:
    return myErrorPage(request)

can be replaced with:

from django.shortcuts import get_object_or_404
myObj = get_object_or_404(MyObject, pk=myObjectID)

The same applies with the second block of code you have.

That in and of itself makes this a lot more elegant.

If you'd like to go further and implement your own decorator, your best bet is to subclass @login_required. If you're passing different arguments or don't want to do that, then you can indeed make your own decorator and it wouldn't be wrong.

ubadub
  • 3,571
  • 21
  • 32
2

1) Yes, chaining decorators is valid as other answers have already pointed out. Good style is subjective, but personally I think it would make your code much harder to read for others. Someone familiar with Django but not your application would need to keep extra context in their head while working with your code. I think it's very important to stick to framework conventions to make your code as maintainable as possible.

2) The answer is yes, it is technically okay to pass in different arguments to the wrapped function, but consider a simple code example of how this would work:

def decorator1(func):
    def wrapper1(a1):
        a2 = "hello from decorator 1"
        func(a1, a2)
    return wrapper1

def decorator2(func):
    def wrapper2(a1, a2):
        a3 = "hello from decorator 2"
        func(a1, a2, a3)
    return wrapper2

@decorator1
@decorator2
def my_func(a1, a2, a3):
    print a1, a2, a3

my_func("who's there?")

# Prints:
# who's there?
# hello from decorator 1
# hello from decorator2

In my opinion, any person reading this would need to be a mental gymnast to keep context of the method signatures at each level of the decorator stack.

3) I would use a class-based view and override the dispatch() method to set instance variables like this:

class MyView(View):
    @method_decorator(login_required)
    def dispatch(self, *args, **kwargs):
        self.myObj = ...
        self.requester = ...
        return super(MyView, self).dispatch(*args, **kwargs)

The dispatch method is what calls your get()/post() methods. From the django docs:

The as_view entry point creates an instance of your class and calls its dispatch() method. dispatch looks at the request to determine whether it is a GET, POST, etc, and relays the request to a matching method if one is defined

Then you could access these instance variables in your get() and/or post() view methods. The advantage of this approach is that you could extract this out to a base class and use it in any number of View subclasses. It is also a lot more traceable in an IDE because this is standard inheritance.

An example of how a get() request would look like:

class MyView(View):
    def get(self, request, id):
        print 'requester is {}'.format(self.requester)
spinlok
  • 3,561
  • 18
  • 27
  • What would the get and post methods in class MyView look like? When is the dispatch() method called? instead of using method_decorator, can you illustrate your answer with decorator1 and decorator2 from above? – Saqib Ali Apr 03 '14 at 04:02
  • Hi @spinlok, if you can answer the question posted in my comment above, I will transfer the bounty to you, since your answer would then be superior to the other answer posted. – Saqib Ali Apr 03 '14 at 21:48
  • @SaqibAli I have added more detail about when the dispatch method is called and how a `get()` method would look like. The `method_decorator` is simply an adapter for using the usual function decorators on methods. My approach doesn't use stacked decorators at all, so I'm not using `decorator2` – spinlok Apr 04 '14 at 00:32
  • OK. This approach would seem to work. But one issue is that I have numerous such views. And in each view, I need to operate on self.requester and self.myObj. So if I copy your dispatch code into each such view, then I'm having to repeat myself many many times, right? That's not DRY. So I guess I have to make a trade-off here between DRYness and maintainability? – Saqib Ali Apr 07 '14 at 02:08