Django Code Smells: ORM Calls

Code Smells

A code smell is a surface indication that usually corresponds to a deeper problem in the system. >- Martin Fowler, coined by Kent Beck

I intend to write a mini series on Django code smells. I highly recommend Martin Fowler’s book: Refactoring and Two Scoops of Django: Best Practices for 1.6 by Daniel Greenfeld and Audrey Roy.

ORM Calls

Django has a reasonable ORM that makes it easy to interact with the database; with little effort you can construct complex queries using the built-in ORM methods. This can lead to pretty hairy code with hidden and duplicated business rules. I like to think of ORM interactions as raw SQL statements and you would not have raw SQL intermingled outside of your model code, would you?

This raw SQL analogy falls apart with simple calls to objects.get(), objects.filter(), etc. For example, I would not argue that it makes sense to go from User.objects.get(pk=123) to User.objects.get_user(123) but for most other interactions, it makes a lot of sense.

But lets take a slightly complicated (and nonsensical) example and cover what is wrong with some ORM interactions:

# myapp/views.py
# ...
adult_staff_users = User.objects.filter(
    userprofile__age__gte=18, 
    Q(is_staff=True) | Q(is_superuser=True),
)
# ...

An altogether confusing example, but nonetheless it is what I would regard as a complex enough query worth tackling.

Let us break down what makes this complex enough:

• The is_staff or is_superuser sounds like it is filtering for staff, so business logic. If we were to always filter for staff this way, what happens if we changed what it means to be staff or added a different staff type? We would need to find all ORM method calls like this and update them.
• We have used the Q object, the from django.db.models import Q import of Q hints at where it should be used. In my opinion, this is certainly not something you should be using in a view.
• The age greater than or equal to 18 also sounds like business logic. In this example it might be filtering for legal adults. What will happen if the age of a “legal adult” changed? We would need to update the code everywhere. What if we added support for a different country with a different idea of what makes someone a “legal adult”? The business logic would need to live everywhere we do this filter.

Instead it is better to pull this filter apart and into the model layer. We might end up with the following pleasant interface for dealing with users:

User.objects.staff().legal_adult()

• The intent of each part of the filter has been encapsulated in these Manager and Queryset methods. A user does not need to guess what attributes on a model makes someone a staff.
• It is now easier to test what a staff user looks like and what makes someone a legal_adult in the context of our system.
• We have documented with code.

Rule: You have models for managing data interactions, use them to encapsulate your data interactions.

• Create queryset methods for interacting with querysets.
• Create model methods to apply business rules to an individual record.
• Create factory management methods to enforce business rules on creation. Similar to how django.contrib.auth.models.User has a create_user management method.
• Create queryset or Manager methods for bulk updates.

You will also find your views, forms and template tags shrink; code will be more testable and data interactions will be standardised across your code base.