0

Consider a room booking system. You might have a Building, Floor, Room models as well as a Booking. We give the room a name based on its building and floor:

class Room(models.Model):
    number = models.PositiveIntegerField()
    name = models.CharField(..)
    floor = models.ForeignKey('Floor')

    def __str__(self):
        return '%s, #%d Floor %d, %s' % (
            self.name,
            self.number,
            self.floor.number,
            self.floor.building.name
        )

This is woefully inefficient when you're doing hundreds of them (eg admin list, big reports, etc.) so I've taken to writing managers like this:

class RoomManager(models.Manager):
    def get_queryset(self):
        return super().get_queryset().annotate(
            roomname=Concat(
                'name',
                V(', #'),
                'number'
                V(' Floor '),
                'floor__number'
                V(', '),
                'floor__building__name',
                output_field=models.CharField()
            ),
        )

And that works. It does everything I wanted it to. It's fast and I've reworked the __str__ to do a if hasattr(self, 'roomname'): return self.roomname before it does the horrendous multi-query string builder.

But now on top of this, I have Booking. Each Booking instance is linked to a single room. There are many cases where to list Bookings, I actually also list room names.

What I've done is write a BookingManager:

class RoomManager(models.Manager):
    def get_queryset(self):
        return super().get_queryset().annotate(
            roomname=Concat(
                'room__name',
                V(', #'),
                'room__number'
                V(' Floor '),
                'room__floor__number'
                V(', '),
                'room__floor__building__name',
                output_field=models.CharField()
            ),
        )

But what the hell? I'm repeating myself. Django is all about DRY and here I am copy and pasting a huge messy annotation around. It's disgusting.

My question is... Is there another way?

Oli
  • 235,628
  • 64
  • 220
  • 299
  • this looks like you are trying to force a view onto a model - if you need a method that returns the full "breadcrumb" of an instance, you could just write a method (on each model) that appends its name on the breadcrumb of its parent - that would recurse down to the base class - why are using queryset annotations ? – MarZab Sep 24 '16 at 21:34
  • if you really need to edit the query set i think you could build the annotation using this method as well.. – MarZab Sep 24 '16 at 21:37
  • If room name is used everywhere, why not just add it as a db column? Floor/building/room number combination doesn't sound like something that would change often if ever. – serg Sep 25 '16 at 18:22
  • @MarZab As I say in the question, there are lots of places where I'm listing. Doing super-deep joins in a prefetch for one field from each layer isn't smart. And it's not a breadcrumb, it's always just the full "address" of the Room. – Oli Sep 25 '16 at 20:40
  • @serg My Building→Floor→Room⋄Booking example is analogous to my actual models that are more abstract and would certainly take some time to explain before I even got to my problem. The intermediate data I'm really dealing with does occasionally change. Denormalising would still be an option but I'm looking for a way to share the annotation aspect of this because even outside this problem, there are a few places where I've written similar, complex annotations multiple times. – Oli Sep 25 '16 at 20:44

2 Answers2

1

While writing this I had an idea, I could write a method that allowed a prefix to be passed in for the relation to room. That way I could call it from anything that was related to room, and it would get the right stuff:

class RoomManager(models.Manager):
    def get_queryset(self):
        return super().get_queryset().annotate(
            roomname=RoomManager.roomname()
        )

    @staticmethod
    def roomname(prefix=''):
        return Concat(
            prefix + 'name',
            V(', #'),
            prefix + 'number'
            V(' Floor '),
            prefix + 'floor__number'
            V(', '),
            prefix + 'floor__building__name',
            output_field=models.CharField()
        )

And in the BookingManager I can just annotate on RoomManager.roomname('room__')

It's cleaner and I'll use it elsewhere but it doesn't feel very clever.

Oli
  • 235,628
  • 64
  • 220
  • 299
0

How about something like this? 

class Room(models.Model):
    number = models.PositiveIntegerField()
    name = models.CharField(..)
    floor = models.ForeignKey('Floor')

    def __str__(self):
        return '%s, %s %s' % (
            self.name,
            self.number,
            self.floor
        )

class Floor(models.Model):
    number = models.PositiveIntegerField()
    building = models.ForeignKey('Building')

    def __str__(self):
        return 'Floor %d, %s' % (
            self.number,
            self.building
        )

class Building(models.Model):
    name = models.CharField(...)

    def __str__(self):
        return '%s' % (
            self.name
        )
MarZab
  • 2,543
  • 21
  • 28
  • I think I just addressed this in the question's comments but specifically, I only ever want the full space address, I just want it in two places (from a queryset of spaces and a queryset of bookings) so I can list them without doing massive joins, or hundreds of thousands (seriously) of recursive queries. Annotations are the way I'm sure is correct for my use, the problem is sharing this annotation between models. – Oli Sep 25 '16 at 20:48
  • Ah, yes I understand now. Well your manager is a good way to reuse that but as far as performance is concerned, @serg has a good point - caching the data is much better than making the database do the work (a bit of cache invalidating will be needed but I guess editing is not that often) - diskspace is cheaper than processing power. – MarZab Sep 25 '16 at 20:58