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

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

Reply via email to