3

A lot of my Django views start a little bit like this:

try:
    # here request.POST could also be request.GET or a captured URL parameter
    MyModel.objects.get(user = request.user, some_attr = int(request.POST['some_val']))
except KeyError:
    # error_view is a view that returns a pretty, formatted 500 response
    return error_view(request, message = 'Incomplete data was submitted (some_val was missing). No action was taken.', next = request.get_full_path())
except ValueError:
    return error_view(request, message = 'The data (some_val = %s) was not formatted correctly. No action was taken.' % request.POST['some_val'], next = request.get_full_path())
except MyModel.DoesNotExist:
    return error_view(request, message = 'The data you were looking for (some_val = %s) could not be found. It is possible you used an outdated form, or the data was just recently removed. No action was taken.' % request.POST['some_val'], next = request.get_full_path())

I find myself repeating code very similar to this a lot (but with different POST/GET/url, filter arguments, data type, next url or messages), sometimes several for one view.

What is the best way to approach this?

  1. Just keep repeating these 8 lines, the code is different enough to not violate DRY
  2. Make a decorator which passes the instance to the view, or returns the error view. Changes the function signature (is that ok?) and not that easy to generalize.
  3. Write a function to do this. In that case, should it...
    1. ... return either a response or an instance, return it if it's a response.
    2. ... return a (instance, response) tuple, oneo f which is None, retuning response if it's not None.
    3. ... raise a special type of exception (say VisibleException), and create a middleware to display it using error_view.
  4. ... something else?

All the methods work but which one do you recommend and why?

I came up with 3.3 just now and it actually makes sense to me - these situations are abnormal exit routes after all - but I don't think I've seen it used before (admittedly I don't know what to search for), so perhaps it has a problem. (The programmer using the app has to add a Middleware, that's one).

EDIT: I was just reminded that get_object_or_404 throws an exception that is kind of displayed to the user, so perhaps 3.3 is a logical extension.

2
  • 1
    (Please let me know if this is more suitable for codereview SE, I really wasn't sure.)
    – Mark
    Mar 14, 2015 at 18:12
  • 1
    Instead of hacking stuff like int(request.POST['some_val']) by hand, use a form with a some_val = IntegerField(required=True) in it. Use the core tools of Django. Apr 20, 2015 at 12:17

1 Answer 1

1

As you pointed out, often the specifics of what constitutes invalid request parameters is different enough for each view, and so including this type of code in each view is (I think) fine and not a DRY violation. It also self-documents the view by having code at the beginning that clarifies what the view expects to receive, either in the URL, GET, or POST, and that's worth something.

However, since there are some situations where very specific patterns might benefit from refactoring, I would defer to the Django code itself for a good guideline on when a decorator (your option #2) or a function (option #3) might be the better option:

  • Django uses view decorators for patterns like login validation, permissions validation, etc. Note that these are situations where the decorator doesn't need to pass results from its processing into the view itself.

  • Django provides functions (rather than decorators) to handle situations like get_object_or_404, get_list_or_404, etc. Note that in these situations, the view does need access to the results of the processing (namely, the instance). Those functions are implemented similarly to your option #3.3 (although I think #3.1 would equally work).

To sum up, option #1 is usually fine and very useful for self-documenting code, but for very specific patterns, determine whether the view needs access to the results. If so, option #3 is probably better (otherwise, option #2). If going with option #3, then 3.3 is the path Django takes, though 3.1 seems equally workable.

Side note:

You mentioned that you are currently returning a 500 response in these situations. A 500 response indicates that the server experienced an error, which is not the case here, and so it's an incorrect response code to deliver.

Instead do one of the following:

  • If your view got a captured URL parameter that is incorrect, then either a 404 should be returned (since that URL doesn't exist), or the view should return a 301 or 302 redirect to a good URL.

  • If the view received GET or POST parameters that are incorrect, then a 400 Bad Request response should be returned.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.