signals pre_save vs model's save()

[Mateusz Harasymczuk]

I am thinking about splitting my model's save() method over few signals.

For example, stripping spaces and making string capitalized.

I have even more sophisticated problems such as making an object field active set to False basing on various parameters from other fields, such as expiration date, or good or bad karma points.

What do you think, it is generally good idea, to keep models file clean out of heavily overloaded save() methods?

How about making more than one signal pre_save to the same model?


@receiver(pre_save, sender=X)
def strip_chars(sender, **kwargs):
    pass

@receiver(pre_save, sender=X)
def capitalize_name(sender, **kwargs):
    pass

@receiver(pre_save, sender=X)
def make_inactive(sender, **kwargs):
    pass

Will it work?

I want to put those in signals/X.py
where X is my model name

Where to import them? in my model file?
or it will happen "automagicly" like with admin.py file?
(I think that python's explicit rule forbids that way, therefore where to import those signals, avoiding recurring imports [signal.py import model.py and model imports signals])?

[Marcos Moyano]

You can set more than 1 [pre,post]_save signal per-model, no problem. I generally, and I'm not saying this is the django way, do it like this:
app_name/
     __init__.py
     modes.py
     views.py
     signals.py

Inside my signals.py I have all my signal declarations and connections.
And inside __init__.py I just do:

import app_name.signals

Rgds,
Marcos
    

--
You received this message because you are subscribed to the Google Groups "Django users" group.
To post to this group, send email to django...@googlegroups.com.
To unsubscribe from this group, send email to django-users...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-users?hl=en.

--
Some people, when confronted with a problem, think “I know, I'll use regular expressions.” Now they have two problems.

Jamie Zawinski, in comp.emacs.xemacs

[Mateusz Harasymczuk]

So it should works.
Great.

my save() methods are much longer than models declaration, and I find it quite a bit confusing to maintain.
and for sure, to tell how it works :}

from PEP 20 -- The Zen of Python:

"

Simple is better than complex.

...

If the implementation is hard to explain, it's a bad idea.

"

[Malcolm Box]

On 28 May 2011 11:05, Mateusz Harasymczuk <mat...@harasymczuk.pl> wrote:

I am thinking about splitting my model's save() method over few signals.

For example, stripping spaces and making string capitalized.

Why would you want to do that?
 

I have even more sophisticated problems such as making an object field active set to False basing on various parameters from other fields, such as expiration date, or good or bad karma points.

What do you think, it is generally good idea, to keep models file clean out of heavily overloaded save() methods?

If this logic is tied to your model, what is the advantage of moving it out of the save() method in your model definition? 
You would more usefully serve the same purpose by decomposing the save() method into several functions e.g.:

def save(self,...):
    self._strip_chars()
    self._validate_and_set_fields()
    super(Model, self).save()

Clean, simple and makes it very obvious what you're doing in the save() method. Compare that with the signals version, which will give someone reading the code no clue at all that when they call save() the values of the fields will change.

How about making more than one signal pre_save to the same model?

It will work, but it's the wrong approach.

Malcolm

[Mateusz Harasymczuk]

W dniu niedziela, 29 maja 2011, 15:36:13 UTC+2 użytkownik Malcolm Box napisał:

On 28 May 2011 11:05, Mateusz Harasymczuk <mat...@harasymczuk.pl> wrote:

I am thinking about splitting my model's save() method over few signals.

For example, stripping spaces and making string capitalized.

Why would you want to do that?

Because I am writing CRM, and my end users are non-technical ladies :}
And each one of them inputs data in different format (id numbers, phone numbers, dates, names [upper cased, capitalized, lower cased])
Therefore I have to normalize an input to store, and then print contract agreements in normalized way.
You may say normalize via template tags at rendering level.
Yes, but I use those data for example to calculate date ranges and text search.
If someone has an resignation addendum I have to make some other changes to model.
It is easier to normalize them in pre_save or at the save() method
 

 

I have even more sophisticated problems such as making an object field active set to False basing on various parameters from other fields, such as expiration date, or good or bad karma points.

What do you think, it is generally good idea, to keep models file clean out of heavily overloaded save() methods?

If this logic is tied to your model, what is the advantage of moving it out of the save() method in your model definition? 
You would more usefully serve the same purpose by decomposing the save() method into several functions e.g.:

my largest model is about 20 fields length.
where save methods are about at least 50 lines long.

it gives me a mess.

and then there are list_display functions (each is 1 to 3 lines long) which almost doubles fields length and gives me a 150-200 lines length model file, with only 20 fileds

and I have not included comments and docstrings...

Clean, simple and makes it very obvious what you're doing in the save() method. Compare that with the signals version, which will give someone reading the code no clue at all that when they call save() the values of the fields will change.

I can provide a comment in a model file, that normalize functions are stored in signals file.

 

How about making more than one signal pre_save to the same model?

It will work, but it's the wrong approach.

I am not saying this is a good approach,
I am thinking this might be a good solution in my case.

Looking forward to hearing some more opinions.
I might reconsider, if someone points me a better solution, than I am thinking of.


--
Matt Harasymczuk
http://www.matt.harasymczuk.pl

[Alexander Schepanovski]

> Because I am writing CRM, and my end users are non-technical ladies :}
> And each one of them inputs data in different format (id numbers, phone
> numbers, dates, names [upper cased, capitalized, lower cased])
> Therefore I have to normalize an input to store, and then print contract
> agreements in normalized way.

Input normalization should be done in form clean() or views.
If it is impossible or inconvenient then Model.save() is ok, but
signals should not handle this.
After all it's your model logic it should be in model, it will be far
more confusing moving it from there.

You can also take a look at Model.clean().

[Malcolm Box]

On 29 May 2011, at 15:53, Mateusz Harasymczuk wrote:

W dniu niedziela, 29 maja 2011, 15:36:13 UTC+2 użytkownik Malcolm Box napisał:

On 28 May 2011 11:05, Mateusz Harasymczuk <mat...@harasymczuk.pl> wrote:

I am thinking about splitting my model's save() method over few signals.

For example, stripping spaces and making string capitalized.

Why would you want to do that?

Because I am writing CRM, and my end users are non-technical ladies :}
And each one of them inputs data in different format (id numbers, phone numbers, dates, names [upper cased, capitalized, lower cased])
Therefore I have to normalize an input to store, and then print contract agreements in normalized way.
You may say normalize via template tags at rendering level.
Yes, but I use those data for example to calculate date ranges and text search.
If someone has an resignation addendum I have to make some other changes to model.
It is easier to normalize them in pre_save or at the save() method

I understand why you might want to clean up data, or have other processing on saving. I have no idea why you'd want to do that using signals rather than making things explicit in the model file.

If this logic is tied to your model, what is the advantage of moving it out of the save() method in your model definition? 
You would more usefully serve the same purpose by decomposing the save() method into several functions e.g.:

my largest model is about 20 fields length.
where save methods are about at least 50 lines long.

it gives me a mess.

Is your problem that you've got 50 lines of code, or that you've got one function that's 50 lines long? If it's the function length that's a problem, then split the function up. Functions (including save()) can call other ones.

and then there are list_display functions (each is 1 to 3 lines long) which almost doubles fields length and gives me a 150-200 lines length model file, with only 20 fileds

If you have a lot of functionality to implement, you will end up with lots of lines of code. The art is in keeping the code organised so that you minimise the amount of code you have to look at at any one point.

and I have not included comments and docstrings...

Well you should probably have some of both :)

Clean, simple and makes it very obvious what you're doing in the save() method. Compare that with the signals version, which will give someone reading the code no clue at all that when they call save() the values of the fields will change.

I can provide a comment in a model file, that normalize functions are stored in signals file.
 

If you don't like having the functions in the model file, then move them into a separate .py file and import them into the models file. There's no need to use signals.

I am not saying this is a good approach,
I am thinking this might be a good solution in my case.

Signals are designed to allow you to react to stuff happening in code that's unrelated to yours - not to allow you to stick a whole bunch of arbitrary processing into the middle of the save() routine for your own models.

Looking forward to hearing some more opinions.
I might reconsider, if someone points me a better solution, than I am thinking of.

Here's the right solution

utils.py:

def capitalise():

   ....

def other_stuff():

   .....

models.py

from utils import capitalise, other_stuff,...

class MyModel(..):

     field1 = models.CharField()

    def save(self, *args, **kwargs):

        self.field1 = capitalise(self.field1)

        self.field1 = other_stuff(self.field1)

        super(MyModel, self).save(*args, **kwargs)

Except for the explicit calls to the capitalise()/other_stuff() functions in the save() method, this code is organised exactly like your proposed signals version, but it has the following advantages:

- It makes it explicit what processing is being done to the model on save()

- It makes the *ordering* of processing explicit - signals don't offer that

By all means go down the signals route if you want, it's your code. But you will then have two problems: your original one, and using signals.

HTH,

Malcolm

[Mateusz Harasymczuk]

That was a very good and convincing post :}

I am testing a solution with .clean(), which IMHO is a better place to clean data before save()

[Malcolm Box]

On 2 June 2011 23:37, Mateusz Harasymczuk <mat...@harasymczuk.pl> wrote:

That was a very good and convincing post :}

I am testing a solution with .clean(), which IMHO is a better place to clean data before save()

Glad to have been of help.

clean() is ideal for cleaning up stuff from forms, so could be a great fit for your use case.

Malcolm