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
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)
Change History (9)
by , 13 years ago
Attachment: | 17182.diff added |
---|
comment:1 by , 13 years ago
comment:2 by , 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 , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
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 , 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"
comment:5 by , 13 years ago
Patch needs improvement: | unset |
---|
comment:6 by , 13 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Though I guess it might be helpful to add explanation that the base
forms.Form.clean
method simply returnsself.cleaned_data
. I'm wary of complicating things though.