On Thu, 2008-04-24 at 03:31 -0700, dimrub wrote: > I don't think having a function with such a profound side effect (such > as making clean_data available) is a good idea. Very counter- > intuitive, IMHO.
It's just a name. Changing it now would impact a lot of existing code (and, as I explain below, you've mis-diagnosed the thing you're concerned with slightly) and we try very hard not to break existing code without really good reason. The name fits very naturally into the normal use pattern, in the sense that code reads quite logically and it's clear what's going on. You call is_valid() to see which path to take (redisplay or do something with the valid data) and in the valid case, the data is already cleaned for you. The side effects aren't particularly profound, since they're entirely contained within the form instance. It's merely changing it's own internal state. You don't even have to know whether it's is_valid() or something else that changes cleaned_data in a logical code flow -- it could well have been __init__ that set that up. It's only if you try to access the normalised data without checking for validity (either by looking at the errors or using is_valid()) that it becomes clear it isn't populated until later and that's not a particularly normal use pattern. Okay, you may not love the name, but it's a bit of an overstatement to say "very counter-intuitive". It's clear to how to use it and whether the side-effect happens in that method or somewhere else (and populating cleaned_data is actually triggered by accessing the 'errors' attribute, which is_valid() happens to do) shouldn't really be a showstopper. Nobody's going to agree with every single name choice in a framework. > Why not split it into two: > > - validate() - that performs the validation and makes clean_data > available Interestingly, we are probably going to introduce a validate() method on forms shortly, along with something like to_python() in order to separate normalisation and validation which fixes some more pervasive little problems. But the intention there is not to replace is_valid() and the normal code flow that exists now will still exist. > - is_valid() - that calls the above validate() and returns its result You've just suggested making is_valid() no longer be a boolean test (since it becomes a data creator). That's making the name bear much less relation to it's use than it currently is. Regards, Malcolm -- On the other hand, you have different fingers. http://www.pointy-stick.com/blog/ --~--~---------~--~----~------------~-------~--~----~ 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 [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-users?hl=en -~----------~----~----~----~------~----~------~--~---
