techscapades

On the SRP: For those unfamiliar with the Single Responsibility Principle (SRP), it states that there should never be more than one reason for a class to change.

That is to say: do one thing, do it well.

Decorators (not to be confused with the decorator pattern) can add behaviors or side-effects to a method, and this can be dangerous. It seems harmless, because by adding a decorator, you’re likely taking boilerplate code and shuffling it elsewhere. However, they often encourage badness because of how easy it is to add these behaviors.

On Testing Decorated Methods:

Adding an @decorator to a python object unarguably makes the undecorated code difficult to test in isolation. Decorators are applied at compile-time, and cannot be mocked or made to not-execute without some pain.

There are certainly a few common tricks that can help test a decorated method with minimal side-effects, but they require changes to the decorator itself. There’s just plain and simple no way to un-decorate a method for isolated testing.

Let’s Look at a Few Common Examples:

@expose: register a URL for a view function in a web framework

class BlogPostController(object):
    @expose("/blog/{post_id}")
    def index(self, request, post_id=None):
        """ show a blog post """
        post = adapter.get_post(post_id)
        return render("show_entry.html", dict(post=post))

Without the @expose, your show_entry knows how to get a given post and render it in the proper template. With the decorator, it also now knows which URL corresponds to that. You now have multiple reasons for this block of code to change, including pointing to a different URL or using a different template. It’s preferred to have a separate module for managing which urls point to which views.

Harm factor: low. Ick factor: medium – high.

@cache: try to get the result from memcache, otherwise, execute the function and stick it in cache for next time

class PostAdapter(DataAdapter):
    @cache
    def get_post(self, post_id)
        """ grab a blog post from the database """
        return self.query(Post).filter(id=post_id).one() # SQLAlchemy folks need to talk to Demeter...

OK this seems cool right? You only hit the database when you have to, otherwise we get it even quicker by looking it up in the cache.

What happens if you want to disable caching? A separate cache abstraction layer would reduce volatility in your data adapter.

And what does the unit test look like?

class TestGettingAPost(object):
    def setup(self):
        self.query = Mock() #don't hit the production database!
        self.post_adapter = PostAdapter(query=self.query)
    def test_getting_a_post(self):
        assert self.post_adapter.get_post(123)

Damn, there’s no way to mock out the @cache decorator so it doesn’t run. Try to, I dare you. You’re likely going to actually get post 123 from your production memcache. Crappy. The only thing you can do is make the @cache grab the cache implementation from the PostAdapter instance (and mock that out in your test), or find some other sneaky way of disabling caching for test runs. But the @cache decorator isn’t all self-contained and fun anymore.

Harm factor: medium – high.

@validate: Make sure the request matches the specified schema, otherwise hand-off to error handler

class EditPostController(object):
    def _save_error(self, request, errors):
        """ @validate decorator kicked flow here, redisplay edit page with errors """
        ...
    @validate(schema, error_handler=_save_error)
    def save(self, request)
        post = self.schema.to_python(request.params)
        self.post_adapter.save_post(post)
        return redirect("/blog/{0}".format(post_id))

Without this @validate, there would be a lot of boilerplate code inside the save method. With it, any unit test for the save method will be likely linked to your schema. You’d have to make an actual valid request in order to test this method. That’s outside of any tests for your schema directly. That means double-coverage but 2 tests to update when requirements shift.

Harm factor: low-medium

Conclusions (or tl;dr):

As easy as it is to become infatuated with Python decorators, they definitely encourage you to violate the SRP. This can create a myriad of problems:

• Difficultly in isolating system under test
• Added complexity to enable testing
• Redundant redundant unit tests
• Making a code module more volatile than it ought to

They still have some valid use cases and can lead to cleaner code, however, as Master Yoda wisely said, “when you look at the dark side, careful you must be…”

6 Responses

1. Aaron Says:
   

   June 4th, 2010 at 11:51 am

   Yeap. I tend to only use decorators after exhausting all other practical options.

   I despair when people act like they don’t cost anything.

2. Gary Bernhardt Says:
   

   June 4th, 2010 at 3:15 pm

   Thanks for writing this post so I don’t have to.

   TurboGears seems to get this more right than others. It uses decorators extensively, but they only add attributes to the function; they never actually wrap it. The result is that a TurboGears controller’s output is easy to test – you just get the template context dictionary when you call the function. No template rendering is even done.

   TG has other design choices baked in that are testing-hostile (global tg.request object, I’m looking at you!), but the decorators are unusually benign, which was a nice surprise for me!

3. Matt Wilson Says:
   June 5th, 2010 at 8:45 am

   Nice post! But I don’t like the subtext that closures in general are sucky. Closures are beautiful.

   The problem comes from code that modifies parameters.

   A decorator like this:

   
@expose("/blog...")
def index


   Is really the same as rebinding the original name to something new, like this:

   
index = expose(index, "/blog...")


   I find it hard to follow anything that works like that.

   So when I want to decorate a function, generally I’ll do it like this:

   exposed_index = expose(index, “/blog…”)

   Now there’s no need for fanciness in the tests. It should be easy to verify the behavior at each layer.

   When I use proper “@” decorators, I’m usually doing something like what Gary is talking about in TG. But in general, the @ syntax gives me the willies.

   PS: In your first example, based on the text afterward, maybe you meant to name the method show_entry. And Demeter is a chick

4. Mike Says:
   June 5th, 2010 at 11:28 am

   Thanks for the comments everyone.

   @Matt: The beauty of closures is what makes decorators so appealing.

   The “decorate as a copy” would certainly be preferred to the “@” syntax, but I still don’t know that it addresses the root of the issue.

   In your example, you still need to test `exposed_index`, because likely that’s what you’ll want consumers of your class to use. What sort of tests would you write? You still can’t mock your imported `expose` method, so in reality you’ll need to assert all the behaviors of index as well as assert that expose did what it ought to (leading to even more test duplication).

   I think decorators and closures become most dangerous as they find their way into the lower layers of code. Near the top (view, presentation), we generally expect code to be more volatile and can sacrifice that for the elegant solutions they give us

5. Matt Wilson Says:
   June 5th, 2010 at 12:19 pm

   After I was confident that my index function worked right, and I was confident that my expose function worked right, I wouldn’t bother writing tests to make sure that I could compose the two.

   The only bug I can imagine showing up after combining expose and index would be due to some interface mismatch; like index returns None and expose expects index to return a dict.

   If that happened, I could just add a new test for index. No need to test the combination of index with expose.

   exposed_index is just an output of expose. If expose’s unit tests are written to cover the whole domain of valid inputs, what could go wrong, except when somebody feeds invalid inputs (e.g., a function that doesn’t follow the protocol)?

6. Arun Says:
   

   June 14th, 2010 at 7:27 am

   Try this link also,

   http://arunnambyar.blogspot.com/2010/05/python-decorators-in-depth.html

   Its highly helpful.
   It is describing a to Z of python decorators..