4

I am using an approach similar to T. Stone's answer on this question. However, I have added an abstract base class, so my models.py looks like this:

class CustomQuerySetManager(models.Manager):
    """A re-usable Manager to access a custom QuerySet"""
    def __getattr__(self, attr, *args):
        try:
            return getattr(self.__class__, attr, *args)
        except AttributeError:
            return getattr(self.get_query_set(), attr, *args)

    def get_query_set(self):
        return self.model.QuerySet(self.model)

class MyModel(models.Model): 
    class Meta:
        abstract = True

    class QuerySet(QuerySet):
        def user(self, pub, *args, **kwargs):
            return self.filter(publisher=pub, *args, **kwargs)

    ...some more methods here

class Book(MyModel):
    title = models.CharField(max_length=100)
    authors = models.ManyToManyField(Author, related_name='book_author')
    publisher = models.ForeignKey(Publisher)
    publication_date = models.DateField()

    objects=models.Manager()
    obj=CustomQuerySetManager() #for testing purposes only, this will override objects later

This allows me to get all of the books for a given publisher like such:

p = Publisher.object.get(pk=1)
Book.obj.user(p).all()

I would like to extend this so I can define a custom query in the Book model then pass a Q object to the QuerySet class, so the query "publisher=pub" can be different for different models. I still want to be able to call this like Book.obj.user(p).all(). Somewhere in the Book model I need:

pubQ=Q(publisher=pub)

Where can I put this and how do I pass it to QuerySet defined in the Abstract Base Class, while keeping the code as DRY as possible?

Community
  • 1
  • 1
AgDude
  • 1,167
  • 1
  • 10
  • 27

1 Answers1

3

That answer is clever, but it breaks the Python principle of "explicit is better than implicit". My first reaction to your code was to tell you that you can't declare a custom queryset inside your model, but I decided to check the mentioned SO answer to see where you got that idea from. Again, it's clever -- not discounting that, but well-written code is self-documenting and should be able to be picked up by any random Django developer and ran with. That's where peer code-reviews come in handy -- had you had one, you'd have instantly got a WTF with that.

The Django core team does it the following way:

class MyQuerySet(models.query.QuerySet):
    def some_method(self, an_arg, another_arg, a_kwarg='some_value'):
        # do something
        return a_queryset

class MyManager(models.Manager):
    def get_query_set(self):
        return MyQuerySet(self.model)

    def some_method(self, *args, **kwargs):
        return self.get_query_set().some_method(*args, **kwargs)

It's DRY in the sense that you don't repeat the actual method definition in the manager. But, it's also explicit -- you know exactly what's going on. It's not as DRY as the method you're referencing, but "explicit is better than implicit". Besides if it's done that way in the actual Django codebase, you can be reasonably assured that it's good practice to do so in your own code. And, it has the side-effect of making it much easier to extend and override in subclasses.

Chris Pratt
  • 232,153
  • 36
  • 385
  • 444
  • Thanks for the response Chris. This accomplishes the same thing as the code in my question, and is certainly much cleaner. It doesn't answer my question about how I can define a Q object at the model level, which will take a single arg from a query in a view. Do you have a suggestion for that? – AgDude Jan 07 '12 at 02:04
  • I have accomplished what I wanted using a classmethod. Thanks for helping me clean up my code. – AgDude Jan 07 '12 at 12:42