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:
- is a feedback view
- so it has the title ‘Contact us’
- and other default metadata
- so it has the title ‘Contact us’
- 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 — “thousands of little classes everywhere and no one knows how to find the places where things really happen”. 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.