Opened 13 years ago

Closed 13 years ago

#17182 closed Cleanup/optimization (fixed)

Best Practice for forms.Form.clean

Reported by: Simon Meers Owned by: Simon Meers
Component: Documentation Version: 1.3
Severity: Normal Keywords: form clean super inheritance
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

https://docs.djangoproject.com/en/dev/ref/forms/validation/#cleaning-and-validating-fields-that-depend-on-each-other

I would advocate changing the example from:

    def clean(self):
        cleaned_data = self.cleaned_data
        # ...

to

    def clean(self):
        cleaned_data = super(ContactForm, self).clean()
        # ...

for the sake of painless form inheritance. Any opposition?

Attachments (2)

17182.diff (737 bytes ) - added by Simon Meers 13 years ago.
17182b.diff (1.3 KB ) - added by Simon Meers 13 years ago.
Added documentation note

Download all attachments as: .zip

Change History (9)

by Simon Meers, 13 years ago

Attachment: 17182.diff added

comment:1 by Simon Meers, 13 years ago

Though I guess it might be helpful to add explanation that the base forms.Form.clean method simply returns self.cleaned_data. I'm wary of complicating things though.

comment:2 by Julien Phalip, 13 years ago

Calling super() is probably good practice, however if we change the code sample, then the documentation around it should also be modified since it refers to self.cleaned_data.

Playing the devil's advocate, though, I'm not sure that this change would add much value in that part of the documentation. I'm wary it could be slightly confusing for beginners. Also it feels that by the time someone gets to deal with form inheritance, then they're not beginners any more and therefore should already know better about how to use super().

I don't feel strong enough either way so I'll let someone else intervene to triage this ticket :)

comment:3 by Karen Tracey, 13 years ago

Triage Stage: UnreviewedAccepted

I think it's worth changing to use super(). For a ModelForm, you need to call the superclass clean() in order to get uniqueness checking done. Yes, we note that in the ModelForm doc, but I think it is often missed. Having the clean() example (even if it's a plain form clean) show the best practice makes sense to me.

comment:4 by Preston Holmes, 13 years ago

Patch needs improvement: set

I would add a very short note about why the call to super - "to maintain validation logic in the parent classes, use super to call the parent's clean method"

by Simon Meers, 13 years ago

Attachment: 17182b.diff added

Added documentation note

comment:5 by Simon Meers, 13 years ago

Patch needs improvement: unset

comment:6 by Claude Paroz, 13 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Jannis Leidel, 13 years ago

Resolution: fixed
Status: newclosed

In [17433]:

Fixed #17182 -- Changed best practice documentation for Form.clean to use super() instead of relying on self.cleaned_data. Thanks, DrMeers.

Note: See TracTickets for help on using tickets.
Back to Top