All Unkept
Posted in: Django, Python, Software development  —  20 November 2010

Class Based Views and Dry Ravioli

Django's new class based views are very cool. I am starting to clean up an existing project using them, and lots of existing views are turning into declarative code. But it makes me worried about the ravioli effect.

Take my feedback form as an example. It is a simple view, and typical of code in my project — a couple of calls to some standard utilities. One of the them, standard_extra_context, adds some standard context variables depending on what it passed in (like title, description etc.) which are used in the HTML metadata and in the page itself. Another, json_validation_request allows a form view to be re-used for AJAX validation. The form declaration and view look like this:

class FeedbackForm(CciwFormMixin, forms.Form):
    email = forms.EmailField(label="Email address", max_length=320)
    name = forms.CharField(label="Name", max_length=200, required=False)
    message = forms.CharField(label="Message", widget=forms.Textarea)

def feedback(request):
    c = standard_extra_context(title=u"Contact us")

    if request.method == 'POST':
        form = FeedbackForm(request.POST)

        json = utils.json_validation_request(request, form)
        if json: return json

        if form.is_valid():
            send_feedback(form.cleaned_data['email'],
                          form.cleaned_data['name'],
                          form.cleaned_data['message'])
            c['message'] = u"Thank you, your message has been sent."
    else:
        form = FeedbackForm()

    c['form'] = form
    return shortcuts.render_to_response('cciw/feedback.html',
                context_instance=template.RequestContext(request, c))

It does one bad thing, namely it doesn't redirect on success, but apart from that it is fine. But there is this annoying boilerplate of flow control. So, very quickly, after creating some standard mixins and base classes that are equivalent to my standard utilities, I now have the following (which now does the redirect-on-success):

class FeedbackForm(CciwFormMixin, forms.Form):
    email = forms.EmailField(label="Email address", max_length=320)
    name = forms.CharField(label="Name", max_length=200, required=False)
    message = forms.CharField(label="Message", widget=forms.Textarea)

class FeedbackBase(DefaultMetaData):
    metadata_title = u"Contact us"

class FeedbackFormView(FeedbackBase, AjaxyFormView):
    form_class = FeedbackForm
    template_name = 'cciw/feedback.html'

    def get_success_url(self):
        return reverse('cciwmain.misc.feedback_done')

    def form_valid(self, form):
        send_feedback(form.cleaned_data['email'],
                      form.cleaned_data['name'],
                      form.cleaned_data['message'])
        return super(FeedbackFormView, self).form_valid(form)

class FeedbackDone(FeedbackBase, TemplateView):
    template_name = 'cciw/feedback_done.html'

feedback = FeedbackFormView.as_view()
feedback_done = FeedbackDone.as_view()

(DefaultMetaData is a TemplateResponseMixin subclass that replaces standard_extra_context, FeedbackBase defines common attributes for FeedbackFormView and FeedbackDone. AjaxyFormView is a FormView subclass and replaces utils.json_validation_request. The last two lines are simply there for the sake of not adding imports to my urls.py).

Now, this is a big improvement in one way. All standard flow control has been removed, so it is very DRY in that sense, and the details shown are the details which are relevant to the one task of handling a feedback form. The way to read this view is as declarative code, similar to how you read models and forms:

FeedbackFormView:

  • is a feedback view
    • so it has the title 'Contact us'
      • and other default metadata
  • is a form processing view (which has been ajaxified)
  • has FeedbackForm as the form
  • has "cciw/feedback.html" as the template.
  • has the 'success url' equal to the 'feedback_done' URL.
  • does the following with a valid form:
    • sends a feedback message.

Only the last of these is really imperative, and this is quite a nice way to think about a chunk of code.

However, suppose we have a maintenance programmer who needs, let's say, to make the template display a certain message on the basis of a certain URL parameter.

In the first case, it is pretty obvious what to do — a couple of lines inserted before the render_to_response call will work fine, with a corresponding change to the template.

In the second, it is also fairly easy to do — you would just have to define an appropriate get_context_data() method. But to find that out, you've got a fairly intimidating stack of mixins and base classes to investigate. In fact, it's more like a tree not a stack (though I think I may already have some diamond inheritance in there by accident).

This is classic ravioli code. As one person on that c2 page says, “this style maximizes maintainability by old hands at the expense of comprehensibility by newcomer”.

Put it this way: suppose you want to go from the current version to the previous version, in a precise and controlled way. You could trace through all the functions that would be called, starting with `as_view()` and working through till you collected all the code paths that were being used, but it would take you a lot longer than the forward transformation just took me. And this is exactly the transformation you might need to do if, for example, you needed to make a fundamental change to the flow control while keeping most of the code intact. It is faster to start from scratch with the declarative reading above — but that will only work if the view really is declarative, and there are no sneaky surprises in the base classes, and even then it may take some digging.

So, what am I saying? Don't do this? I guess I'm just saying “be aware of this trade-off”. As Shannon Behrens recently blogged, there are times when fixing DRY violations are not worth it.

Comments §

§ On 20 November 2010, Ziming wrote: 959
Indeed, 1 of the troubles I faced with the new class-based generic views in django trunk is being unsure of which methods to overwrite. Took me quite a while of documentation reading as well as browsing through the source code.

§ On 20 November 2010, bne wrote: 960
But ravioli is flat!
Surely this is spaghetti, tagliatelle or
even papardelle code?

;)

§ On 20 November 2010, Batiste Bieler wrote: 961
I have been the victim of this exact phenomenon on a Django. A bunch of ugly but quite straightforward views function have been transformed into a hierarchy of class and mixin.

The result end up being quite difficult to understand to new comers and myself. The maintenance and debugging suffered too because of the deep hierarchy that followed.

Be careful with Class based view. Use them with moderation and keep the inheritance simple.

§ On 20 November 2010, luke wrote: 962

@Ziming: I too found it much easier to browse the source than to read the docs for this. I don't think it is because of poor documentation, either. OO interfaces are just much harder to describe than functional ones.

I meant to point out that another way to use the new class-based views is by composition and wrapping rather than inheritance - i.e. use the result of SomeView.as_view() as a simple view function. This allows you to convert the OO interface into a simple functional one.


§ On 21 November 2010, Ziming wrote: 963
@luke: Well I don't blame the Django documentation for making me look into the source code for the sequence of methods called. After all that an advantage of being open source. We could still look into the source code as additional documentation (:.

However I still feel the documentation could better benefit from giving us a trace of the more important methods that are called from the first to the last.

§ On 22 November 2010, Ole Laursen wrote: 964
Yeah, now your code is not in control anymore, the framework is and you have to understand a number of concepts to guess what happens. I think you're going to have trouble maintaining it yourself a year from now.

Also, you forgot to mention that the new approach appears to be just about the same number of lines as the old one. :)

Another way to achieve a more declarative approach to the standard page things could be deriving a class for that, only, then use it in your functional view.

I do something a bit like that for most complex sites I've built. Instead of using template inheritance and having to stuff a lot of things into contexts and template tags, most of my views simple render a <div> with their content, then end with a return render_page(page, divcontent) which takes the page configuration (title, menu, etc.) and the <div> produced by the view and constructs the final complete HTML page.

§ On 5 December 2011, Neil wrote: 1309
Yea good point. I often find class based views are more complex and harder to read as they can be more verbose. Keeping things simple should definitely factor in to the decision of choosing class based views.

Add comment

Format:

  • Javascript has to be on to get past my spam protection, and cookies, and there is a delay, sorry for any inconvenience!
  • I reserve the right to moderate comments.