views:

137

answers:

1

I'm doing something that doesn't feel very efficient. From my code below, you can probably see that I'm trying to allow for multiple profiles of different types attached to my custom user object (Person). One of those profiles will be considered a default and should have an accessor from the Person class. Storing an is_default field on the profile doesn't seem like it would be the best way to keep track of a default, is it?

from django.db import models
from django.contrib.auth.models import User, UserManager


class Person(User):

    public_name = models.CharField(max_length=24, default="Mr. T")

    objects = UserManager()

    def save(self):
        self.set_password(self.password)
        super(Person, self).save()


    def _getDefaultProfile(self):

        def_teacher = self.teacher_set.filter(default=True)
        if def_teacher: return def_teacher[0]

        def_student = self.student_set.filter(default=True)
        if def_student: return def_student[0]

        def_parent  = self.parent_set.filter(default=True)
        if def_parent:  return def_parent[0]

        return False
    profile = property(_getDefaultProfile)


    def _getProfiles(self):
        # Inefficient use of QuerySet here. Tolerated because the QuerySets should be very small.
        profiles = []
        if self.teacher_set.count(): profiles.append(list(self.teacher_set.all()))
        if self.student_set.count(): profiles.append(list(self.student_set.all()))
        if self.parent_set.count():  profiles.append(list(self.parent_set.all()))

        return profiles
    profiles = property(_getProfiles)




class BaseProfile(models.Model):

    person = models.ForeignKey(Person)
    is_default = models.BooleanField(default=False)

    class Meta:
        abstract = True


class Teacher(BaseProfile):
    user_type = models.CharField(max_length=7, default="teacher")


class Student(BaseProfile):
    user_type = models.CharField(max_length=7, default="student")


class Parent(BaseProfile):
    user_type = models.CharField(max_length=7, default="parent")
+1  A: 

First of all you could make things a lot more easy by not declaring the BaseProfile abstract:

from django.db import models
from django.contrib.auth.models import User, UserManager

class Person(User):
    public_name = models.CharField(max_length=24, default="Mr. T")
    objects = UserManager()

    def save(self):
        self.set_password(self.password)
        super(Person, self).save()

    def _getDefaultProfile(self):
        try:
            return self.baseprofile_set.get(default=True)
        except ObjectDoesNotExist:
            return False
    profile = property(_getDefaultProfile)

    def _getProfiles(self):
        return self.baseprofile_set.all()
    profiles = property(_getProfiles)

class BaseProfile(models.Model):

    person = models.ForeignKey(Person)
    is_default = models.BooleanField(default=False)    

class Teacher(BaseProfile):
    user_type = models.CharField(max_length=7, default="teacher")    

class Student(BaseProfile):
    user_type = models.CharField(max_length=7, default="student")    

class Parent(BaseProfile):
    user_type = models.CharField(max_length=7, default="parent")

The way this is nicer? Your properties didn't know anyway what type they were returning, so the abstract baseclass only made you have an incredible annoying overhead there.

If you now are wondering how the hell you can get the data from the specific profiles since I made anything returned BaseProfile? You can do something like this:

try:
    #note the lowercase teacher referal
    print myuser.profile.teacher.someteacherfield 
except Teacher.DoesNotExist:
    print "this is not a teacher object!"

Also I do hope you didn't use the user_type field solely for this purpose, because django has it built in better as you can see. I also hope you really have some other unique fields in your derived profile classes because otherwise you should throw them away and just past a usertype field into BaseProfile (look at choices to do this good).

Now as for the is_default, imho this method is as good as any. You can always try to add custom constraints to your dbms itself, saying there sould be 0 or 1 records containing the same FK and is_default=True (there is no django way to do this). What I also would say is, add a method make_default and in that method make sure the is_default is unique for that person (e.g. by first setting is_default to False on all profiles with the same FK). This will save you a lot of possible sorrow. You can also add this check in the save() method of BaseProfile.

Another way you could do it is by adding a Foreign Key to the Person Model that points to the default Profile. While this will ensure default to be unique on django level, it can also provide denormalization and corruption of your data, even on a more annoying level, so I'm no big fan of it. But again, if you do all adding/removing/updating of profiles through predefined methods (will be more complex now!) you should be safe.

Finally, maybe you have good reasons to inherit from User, but the default way to extend the User functionality is not this, it's described here.

KillianDS
I had avoided removing the abstraction from the BaseProfile in hopes to not have to traverse through another layer before reaching my 'typed' profiles while in the templates. However, the missing piece to that puzzle I missed was creating accessors on the model that obscured that layer from the templates. Now I see that going this route opens up a host of better ways to do this, which is exactly what I was looking for. I will experiment with this approach, thank you Killian!
Scott Willman