Wellfire Interactive // Expertise for established Django SaaS applications

Refactoring - like washing your hands in the bathroom (This Old Pony #59)

Do you schedule time to refactor your Django app? 

Uncle Bob on Refactoring

This quote from “Uncle Bob” Martin (of “Clean Code” and Agile Manifesto fame) got me thinking about strategies for _approaching _refactoring, to say nothing of issues around what _is _refactoring and why it’s good practice.

Of course, there’s a disconnect between _is _and _ought _as Josh points out below:

How people really approach refactoring...

Aside from being hilarious and [sadly] true, it paints a nice picture of what happens with refactoring: usually not much.
 

What is refactoring and why is it good

The canonical definition of refactoring is changing code in such a way that improves non-functional aspects of the code. _Renaming _is one of the most straightforward techniques for refactoring code, but it also includes breaking apart functions and methods, sometimes even class, or creating all new ones. 

There are arguments about what constitutes refactoring. Much of what developers talk about with regard to refactoring is well beyond what most of the authors on refactoring would admit as refactoring. Rewriting and redesigning are important tools for improving parts of codebases, but they’re well beyond refactoring.

Refactoring though is a good practice because it improves the code, obviously, right?. But it’s important, making these non-functional changes, because most working software is always changing. Features change, existing features are reused in different ways, a one-off class becomes a critical part of the codebase… Refactoring makes it easier to understand both what the software does, what it is intended to do, and makes it easier to work with it in a reliable way.
 

When to refactor?

When you’re working with the code in question!

That’s it. There’s no hard rule one way or the other, but it’s much like how to go about writing tests. If you have untested code you write tests for a block of code (or a function or a class) when you touch that block of code (or function or class).
 

Some modest suggestions for refactoring your Django app

If you’re unsure about what you’d refactor or where to start then use this list as your inspiration.
 

Querysets

  • If you’re writing long filter expressions in views, etc, encapsulate these in a named manager/queryset method.
  • If you’re writing long expressions with multiple sequential conditions, separate these into distinct queryset methods and chain these. It can sound dumb at first when you have only one know use case, but separating out the conditions into named blocks often goes a long way to making it clear what exactly the code is filtering and why
  • Use manager methods to return other data pulled from querysets even if that data is not a queryset itself (like building data for export), providing one super simple to use and test interface

Management commands

  • Break up the logic in commands. Because they often start life as one-offs, management commands especially get big and gnarly.
  • Break _out _logic in commands, treating the command class like it was meant to be treated - as an interface

Model methods

  • Rename or add model methods for logical decisions based on a model instance’s state, e.g. “can_ride_rollercoaster()”. Like with the chained querysets this can feel silly at first when the logic itself is so simple, but it makes the rest of the code cleaner and when you do have to make a change - you will! - it’s significantly safer
  • Break out logic for validating data on save. Yes you can put all of this in the model’s “save” method but it’s clearer if you break it out. You get other benefits, too, if you’re performing custom validation prior to save when using ModelForms[2]

URLs

  • If you have apps with different URL patterns, e.g. one for the public facing site and one for a custom admin dashboard, instead of including one set all individually somewhere define a specific URLs module, e.g. “dashboard_urls.py” and include that where needed.
  • Using the “url” tag in templates a lot? You’re probably used to using get_absolute_url() because Django uses it internally, but there’s no reason you can use additional methods for other important and frequently used URLs. This can clean up templates (more below!)

Templates

  • Better base template usage: make use of base templates. If you’re repeating too much markup, consider moving it into a base template. Or if you can’t understand your template inheritance stack then maybe collapse a few of these and use includes[3]
  • Better use of includes: take snippets, even if used in one or two templates, and move to an include especially for stuff like including third-party JS. This is one of those cases where it makes reading everything much easier, and with cached templates in production you won’t notice any performance hits.
  • Use “with” for cleaner templates: make good use of this template tag[4] so that long attribute chains are not sprinkled around.
  • Move logic up the stack: if you can push it from the template up into the view, and potentially beyond, this is often a win for both template readability as well as performance.

Tests

  • Check for unnecessary DB usage (e.g. testing model methods that don’t rely on a primary key or foreign keys) as an opportunity to speed up tests. Those are just a few ideas to start. I didn’t mention the _names _of these things, which is applicable for pretty much any category here, from model methods to template files. Where else do you see in your app that requires refactoring?

Encapsulatedly yours,
Ben

[0] https://twitter.com/unclebobmartin/status/1024254121338126336
[1] https://twitter.com/CodingItWrong/status/1024290384250331136
[2] Model instance cleaning: https://docs.djangoproject.com/en/2.1/ref/models/instances/#django.db.models.Model.clean
[3] The idea of template inheritance is a huge improvement over having to rely on includes for each rendered page to pull of the other necessary content in. However in some cases just using a conditional include makes more sense than a layer of thin base templates.
[4] with: https://docs.djangoproject.com/en/2.1/ref/templates/builtins/#with

Learn from more articles like this how to make the most out of your existing Django site.