views:

67

answers:

3

Lets say that we are getting POSTed a form like this in Django:

rate=10
items= [23,12,31,52,83,34]

The items are primary keys of an Item model. I have a bunch of business logic that will run and create more items based on this data, the results of some db lookups, and some business logic. I want to put that logic into a save signal or an overridden Model.save() method of another model (let's call it Inventory). The business logic will run when I create a new Inventory object using this form data. Inventory will look like this:

class Inventory(models.Model):
    picked_items = models.ManyToManyField(Item, related_name="items_picked_set")
    calculated_items = models.ManyToManyField(Item, related_name="items_calculated_set")
    rate = models.DecimalField()
    ... other fields here ... 

New calculated_items will be created based on the passed in items which will be stored as picked_items.

My question is this: is it better for the save() method on this model to accept:

  • the request object (I don't really like this coupling)
  • the form data as arguments or kwargs (a list of primary keys and the other form fields)
  • a list of Items (The caller form or view will lookup the list of Items and create a list as well as pass in the other form fields)
  • some other approach?

I know this is a bit subjective, but I was wondering what the general idea is. I've looked through a lot of code but I'm having a hard time finding a pattern I like.

Clarification:

OK, so the consensus is that it should go into a different function on the model, something like inventory.calculate(...) which will then create everything, do the business logic, etc... That's good to know. My question remains: where is the best place to look up the form data into db objects? Should the caller of this function convert primary keys to database models or should the model methods accept primary keys and do it themselves? It's something that I want to do the same way project-wide.

Clarification 2:

OK so now there is some disagreement as to wether overriding save is ok or not.

When you get a form submission for a simple CRUD type operation, you pass the models and values as arguments to Model.objects.create(...) or override save or use signals or whatever.

I think the core of my question is this:

If the form submission has related models used for business logic, then you need to write some business logic into your model layer. When you do this, where should it go and should that method accept a list of objects or a list of id's? Should the model API's accept objects or id's?

A: 

Leave the save() method alone. Create a new classmethod on either the model or the manager instead.

Ignacio Vazquez-Abrams
Thanks for the comment regarding not overriding the save method. The core of my question though is when writing business logic in a model function, is it a better practice to accept primary keys and do the lookup inside the models, or is it better to expect the caller to lookup the objects by primary key and pass a list of objects in?
poswald
Pass the manager method the field values and a list of IDs and let it create and return the new model.
Ignacio Vazquez-Abrams
A: 

Don't override save() method; it's against the convention. Just create the items outside and call save like you do for any other model.

inv = Inventory(rate = 12.3)
inv.calculated_items.add(item1)
inv.calculated_items.add(item2)
item3 = Item(name = "blah")
item3.save()
inv.picked_items.add(item3)
inv.save()

You can declare another function and encapsulate this code into that function.

Amarghosh
That overriding save() is against the convention is new to me. Can you point me to some document that says so? All I could find in the docs was: "You're free to override these methods (and any other model method) to alter behavior." http://docs.djangoproject.com/en/dev/topics/db/models/#overriding-model-methods Thanks!
Daniel Hepper
-1 Overriding save is perfectly acceptable. Make sure you when you do that you pass in *args and **kwargs though (see Daniel's doc link). eg. def save(self, *args, **kwargs):
DrBloodmoney
A: 

OK so the first two answers I got have now been contradicted by others. I've been researching this and I'm going to take a stab at answering it myself. Please vote if you think this is correct and/or comment if you disagree with my reasoning.

  • Methods on models should accept objects and lists of objects, not ids as int/long or lists of id's or anything like that. This is because it will probably be called from a view or form and they have access to full objects from cleaned_data dict. The create() method on manager classes are another example where django itself accepts objects.

  • The caller of the model layer methods should look up and convert the id's into full objects.

  • You can override save() but if you do, you should be careful to accept args and **kwargs

  • If the models span applications, you should consider signals instead of overriding save

  • Don't try to get clever overriding the model manager create method. It wont get called if the view layer creates a new object and saves it. If you need to do extra processing before the save, you can override save or __init__ or catch a signal. if you override __init__ you can check for a pk to determine if it exists in the db yet or not.

I'm going to put my create code in a separate method for now until I figure out which of the techniques I like best.

I think this is a good set of guidelines for adding methods to the model layer. Anything I missed?

poswald