3 ms·
The main point raised by the article is spot-on, and I'm ashamed to say that I had never recognised it as an issue before reading it. It applies even more stron
by Aramgutang 14y ago
The main point raised by the article is spot-on, and I'm ashamed to say that I had never recognised it as an issue before reading it. It applies even more strongly for more complex lookups (possibly involving Q objects), which I've always felt would find a better home in models.py than in views.py. And I too cringe every time I come across the django.db.models.manager source code.
Some thoughts:
The approach goes slightly against the commandment of "there should just be one way of doing it". It's probably best to apply it sparingly, only where there are very clear readability and/or DRYness improvements to be had.
The `PassThroughManager.for_queryset_class(TodoQuerySet)()` bit is a bit intimidating, especially to someone not familiar with django-model-utils. I'd probably take the time to write a `manager_with_queryset(queryset, manager=models.Manager)` function to make things more readable.
When you're trying to convince someone that the way they currently code isn't optimal, it really helps if the code you use to illustrate the current way looks like something your reader is likely to write. The strangely formatted filter chain at the start is thus really off-putting. Instead of adding three disclaimers asking the reader not to focus on the implementation details, why not just write it the way most people would:
todos = Todo.objects.filter(
owner=request.user,
is_done=False,
priority=1,
)
No need to make the current way of doing things seem unnecessarily convoluted, the point you're making still stands. Though I'll admit that when you're forced to throw an .exclude() into the chain, it starts looking a lot like your example.
- randlet 14y agoAgreed. I found the contrived chained filters at the beginning of the article off putting but the rest of the article was quite interesting.
- richardlblair 14y agoI completely agree, but I think this was author's way of demonstrating how ugly a call to the ORM can get. With his very simple to-do list app I think this was the easiest way for him to do that. In real life you would obviously have all your filter in one call (most of the time), then perhaps .values call with an .annotation call in there too.
- ryankask 14y agoI've been using django-model-utils for a while and really love it. I contributed the patch for PassThroughManager that adds the for_queryset_class. I agree that it's not pretty, but there's a bit of history that made it that way. There are lot of custom QuerySet snippets floating around. One was `manager_from` by George Sakkis which Carl included in django-model-utils in July 2010. It was great except that the QuerySets it returned couldn't be pickled. It is currently pending deprecation. It was replaced by Paul McLanahan's PassThroughManager. You used it like objects = PassThroughManager(MyQuerySet) That looks great except when related managers are instantiated, they aren't passed MyQuerySet (I haven't looked at the code in while and you have to dig around, but check out https://github.com/django/django/blob/master/django/db/models/fields/related.py#L448 https://github.com/django/django/blob/master/django/db/model...). You can still use it the old way but if you had an `alive` method on your QuerySet, you couldn't do: home.occupant_set.alive()