Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Building a higher-level query API: the right way to use Django's ORM

dabapps.com

Post reply on HN
Print view View original

21–30 of 30 posts

Re: Building a higher-level query API: the right way to use Django's ORM

#21

Post by simonw »

zacharyvoase wrote: post #10
j4mie wrote: post #9

Earlier quoted context omitted.

Yep, and my criticism of your suggested approach wasn't intended to be particularly strong by any means. I could definitely be sold on the idea. It just felt like a workaround to a problem that could probably be solved in a nicer way. I think my main objection is that these query methods should conceptually be on the QuerySet, and so defining them on the Manager (the "wrong place") and magically copying them to the Q…

But my problem is that most people wouldn't even think of subclassing QuerySet. When we write methods that operate on collections of things, we typically use @classmethod. Without @classmethod, we'd have to write a custom metaclass (and instruct our class to use that) if we wanted even a single class method on a class. Multiple inheritance would break (or at least be difficult to reason about) when classes defined cl…

A few years ago I was experimenting with exactly this problem, and I came up with an API that worked using an inner class on the model. I can't find the code now, but from memory it looked something like this:

    class BlogEntry(models.Model, MagicManagerMixin):
        title = models.CharField(max_length = 128)
        is_published = models.BooleanField(default = False)

        class QuerySet(models.QuerySet):
            def published(self):
                return self.filter(is_published = True)

    entries = BlogEntry.objects.published()
Where MagicManagerMixin was some scary code that made sure the objects Manager would use the queryset subclass.

Re: Building a higher-level query API: the right way to use Django's ORM

#23

Post by jpwagner »

lojack wrote: post #20
jpwagner wrote: post #18

Wait, what's wrong with: class Todo(models.Model): content = models.CharField(max_length=100) # other fields go here.. @classmethod def incomplete(cls): return cls.objects.filter(is_done=False) @classmethod def high_priority(cls): return cls.objects.filter(priority=1)

There's a couple reasons that's not ideal. The big one is that you'd lose filter chaining. With your example you couldn't do Todo.objects.high_priority().incomplete() The other issue is a semantic one. In your example you could do: Todo.objects.all()[0].incomplete() which will return a QuerySet of all incomplete Todo items. This, at least to me, doesn't make sense. The last reason is that by using a Manager you are e…

No, I believe you've read the code above incorrectly. This would be, for example:

  Todo.high_priority().all()
and would allow, for example:

  Todo.high_priority().filter(id__gte=1)
I haven't tested chaining these, but this might work:

  Todo.high_priority().incomplete().filter(id__gte=1)

Re: Building a higher-level query API: the right way to use Django's ORM

#25

Post by j4mie »

jpwagner wrote: post #23
lojack wrote: post #20

Earlier quoted context omitted.

There's a couple reasons that's not ideal. The big one is that you'd lose filter chaining. With your example you couldn't do Todo.objects.high_priority().incomplete() The other issue is a semantic one. In your example you could do: Todo.objects.all()[0].incomplete() which will return a QuerySet of all incomplete Todo items. This, at least to me, doesn't make sense. The last reason is that by using a Manager you are e…

No, I believe you've read the code above incorrectly. This would be, for example: Todo.high_priority().all() and would allow, for example: Todo.high_priority().filter(id__gte=1) I haven't tested chaining these, but this might work: Todo.high_priority().incomplete().filter(id__gte=1)

Your last example wouldn't work. Todo.high_priority() returns a plain QuerySet, which won't have your "incomplete" method (as that's defined on the Model class in your example).

Re: Building a higher-level query API: the right way to use Django's ORM

#26

Post by jacobian »

legutierr wrote: post #12
j4mie wrote: post #9

Earlier quoted context omitted.

Yep, and my criticism of your suggested approach wasn't intended to be particularly strong by any means. I could definitely be sold on the idea. It just felt like a workaround to a problem that could probably be solved in a nicer way. I think my main objection is that these query methods should conceptually be on the QuerySet, and so defining them on the Manager (the "wrong place") and magically copying them to the Q…

Why not just pass a QuerySet implementation into models.Manager as an optional argument? This would be much cleaner, I think: class QuerySetSubclass(QuerySet): def new_method(self): pass class Model(models.Model): objects = models.Manager(QuerySetSubclass)

This is the approach I'd like to see; I'll be proposing it for addition in Django 1.5.

Re: Building a higher-level query API: the right way to use Django's ORM

#27

Post by lojack »

jpwagner wrote: post #23
lojack wrote: post #20

Earlier quoted context omitted.

There's a couple reasons that's not ideal. The big one is that you'd lose filter chaining. With your example you couldn't do Todo.objects.high_priority().incomplete() The other issue is a semantic one. In your example you could do: Todo.objects.all()[0].incomplete() which will return a QuerySet of all incomplete Todo items. This, at least to me, doesn't make sense. The last reason is that by using a Manager you are e…

No, I believe you've read the code above incorrectly. This would be, for example: Todo.high_priority().all() and would allow, for example: Todo.high_priority().filter(id__gte=1) I haven't tested chaining these, but this might work: Todo.high_priority().incomplete().filter(id__gte=1)

Sorry, my example was explaining how it would be done if you were using Managers. Using your example:

    Todo.high_priority().incomplete()
would fail because the QuerySet doesn't have an incomplete() method.

Re: Building a higher-level query API: the right way to use Django's ORM

#28

Post by pandyashreyas1 »

Momentarily arguments sounds right(perhaps because it's intellectually appetizing) but I think we are forgetting basic philosophy of every layered architecture that "lower layer provides generic api to its higher layer" allowing higher layer to customize its every possible needs using this api. Django ORM does exactly same thing.

author seemed to be concerned about these issues:

1. embedding businnes logic in views:

making query isn't business logic, business logic is in your database constraints or sometimes if db constraints are not enough then by overriding save() and validate(). queries belong in views because all we are suppossed to do in views is mearly fetch data from a data structure(already modeled according to biz. logic) and representing it as we see fit(thus the name views). theoretically this representations(views) could be of infinite types and changes over time so queries would change for every representation and over time but we can't go about implementing all possibilities in models. And this is what's antipattern because we are talking abaout putting views in models(partially though).

2. code reusability:

agreed, some queries could be repeted many a times and if complicated enough may clutter the code. I recommend putting querries into functions and put the functions in views or any similar aproach(I will think of one or you figure out one and share) but they just dont belong in models. although I believe full reusabilty can be achieved but in some cases if we can't- well we are choosing 'division of functionality' over 'reusability'.

and most important of all if django's documentaion does not suggest inherting for eg. queryset class then we shouldn't (even if you yourself coded the django framework) because these implementation details are supposed to be concealed and hence they are free to change it in future versions making our code 'upgrade-ugly'(if that's the right term)

Re: Building a higher-level query API: the right way to use Django's ORM

#29

Post by JoshMock »

Lately I've been re-discovering the value of the ORM and putting as much business logic on your models as possible. This, after spending way too long writing out lots of query logic in views instead. It's amazing how you can many times reduce complexity from 10-20 lines to 2-3 lines, and gain reusability, just by putting business logic where it belonged in the first place.

Re: Building a higher-level query API: the right way to use Django's ORM

#30

Post by pestaa »

JoshMock wrote: post #29

Lately I've been re-discovering the value of the ORM and putting as much business logic on your models as possible. This, after spending way too long writing out lots of query logic in views instead. It's amazing how you can many times reduce complexity from 10-20 lines to 2-3 lines, and gain reusability, just by putting business logic where it belonged in the first place.

    > putting business logic where it belonged in the first place
I've been thinking a lot about where to put business logic, and I think the models are the closest, but not the best place for them.

A significant portion of this logic for me affects more than one model, and while you could solve this by using public interfaces, you still need to put in any of the models, which seems like a suboptimal approach.

The ORM is already responsible for several layers, and custom business logic is not relevant there imho.

Still puzzled how to organize my code. MVC has started to fall apart for me. Just my two cents.

Post reply on HN