Saturday, January 24, 2009

Render, Dammit

I choose to believe that this little anti-pattern is not unique to my company...

Please Do Not Do This At Home or ANYWHERE ELSE!

Consider, if you will, a form. You may imagine a simple little form. We tend to prefer big, honking ones in our ENTERPRISE CLASS APPLICATION. Any old form will do for purposes of this discussion.

The form submits to a controller action (e.g. create or update). Should the action recognize an error, we note the error in the session flash, then redirect back to the submitting form.

Not only do we do this (a lot), but we have built up several conventions around it. We have a :messages_top and :messages_bottom that put the warning at the top or bottom of the form. We even have many, many ways to store the submitted params in the session so that the form can show the submitted values when displaying the error message.

We have spent so much time doing this that it actually works. Well, most of the time. Except when the user clicks on an FAQ link, then the back button and all the data (in the session) is lost. Or when the Rails full error messages do not read quite right (why didn't the Rails people make messages read better?).

The correct way of handling this, of course, is to have the action not redirect when an error occurs or a validation fails. Instead, the action should re-render the submitting form template. The action already has the error messages, the submitted form parameters and can easily build up any lists that are needed in the form.

How on Earth did this Happen?

Quite simply, this is the work of one or more NNPPs.

One instance of this can be forgiven. If you are new to the framework and web development and you are under the pressure of a deadline, it happens. But to not go back and question if there is a better way, to never come across a better way in reading a book or even the scaffolding, is unacceptable.

Ultimately, the team needs to tell the developer, "please stop, you're hurting us". If the NNPP fails to grow into a NPPP, the developer needs to go.

2 comments:

  1. I think the key here is not whether or not storing the params in the session on error and redirecting is a good idea in Web/HTTP programming in general, but that doing so with in the Rails framework is clearly far from idomatic. In other frameworks, such as Seaside, they probably do in fact redirect on all posts, even errors, because the framework may be built to support that idiom. I'm not sure if it is, I'm just using it as an example to prove a point. Any time you are building entire mini-frameworks within the framework you are working in, especially a public, open-source, widely used framework such as Rails, you are doing it wrong.

    ReplyDelete
  2. Good point, thanks!

    This was definitely a case of not following the framework idiom. Still, if you're going to be storing params and state in the session, your framework better have support for continuations or some equivalent. Otherwise you'll find yourself building up mini-frameworks that almost work.

    ReplyDelete