Alex Whittemore
Alex Whittemore

Reputation: 1015

Are there pitfalls of adding methods to a class at runtime instead of subclassing?

As of Django 1.5, there's now built-in support for subclassing the User model to add custom functionality, but there are still some considerations of compatibility that mean I'd prefer not to. And prior to 1.5, subclassing was a big hassle. However, there's some functionality I need, specifically in the form of a method, .full_name(). This returns the first + " " + middle + " " +last name of a User. More generically, it only places spaces where appropriate, and can handle a middle name but doesn't require that the model contain one.

Now, rather than subclass User, I simply have the following at the top of my models.py:

def full_name(self):                                    # Here's a useful full_name method
    '''Return the full name of an object, or else an empty string. Can be applied to objects with first_name, last_name and optionally middle_name fields'''
    fullname = self.first_name
    try:
        self.middle_name
    except AttributeError:                          # this type of object might not have a middle_name field
        pass
    else:                                           # If it has a middle_name field, append it with a conditional space
        if(self.first_name and self.middle_name):
            fullname += " "
        fullname += self.middle_name
    if(fullname and self.last_name):                # if fullname has stuff in it now, we'll need a space.
            fullname += " "
    fullname += self.last_name
    return fullname
User.full_name = full_name                              # Bless the User model with our full_name function

I'm relatively new to Python in general, so I was quite pleased to find that this works. I saved a bit of time by using this same function for a couple other models (simply describing full_name = full_name in their class definitions later in models.py). Perhaps now the middle_name functionality makes more sense - other models have middle names.

My question is, is there inherently anything wrong or bad about this practice vs subclassing? Presumably the main disadvantage is that I couldn't add fields to a model this way. But is that it? This method addition appears to persist throughout the rest of the app, and any other module that imports User (specifically, templates work as expected).

Basically, I think this is a quite clever and elegant (though I'm sure not novel) way of solving my problem of needing more functionality, but is it going to come back and bite me later?

Further detail: One reason I ask is that I have some user roles (let's call them Painter, Clerk, and Manager) with models.ForeignKey(User, blank=True) relationships. So for example, User.painter may return a Painter role, but it also may raise an ObjectNotFound exception if the user is not, in fact, a painter. Rather than litter my code with try: except: statements, I'd like to expand my use of the above idea to add, for example, .is_painter() and .is_manager() methods to User so I can simply query those in an if statement instead of making little try:except: messes everywhere.

Upvotes: 0

Views: 61

Answers (1)

Sean McSomething
Sean McSomething

Reputation: 6517

This is referred to as monkey patching. As you may have gathered from it's less-than-flattering name, not always looked upon favorably. In the Python community, it's not a common technique, generally reserved for when there's no other option. Since you have a proper interface (subclassing) to achieve what you want, it's best avoided.

Monkey patching can be fragile, breaking if the module you're modifying changes underneath you. It can be hard for somebody else to modify/debug later because it's not always obvious where the code actually lives.

That said, if it's already there & working, you can keep it in. Nobody's going to track you down and beat you in a dark alley for it.

Upvotes: 1

Related Questions