Opened 13 years ago

Closed 11 years ago

#15616 closed Cleanup/optimization (needsinfo)

django.core.exceptions.ValidationError is poorly implemented and documented

Reported by: tomevans223 Owned by: nobody
Component: Core (Other) Version: dev
Severity: Normal Keywords: validation validationerror
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django.core.exceptions.ValidationError has a horrific interface. It's publicly accessible attributes differ depending upon how the object is created, as do the pre/post conditions for it's publicly accessible methods.

If constructed with a dictionary of error messages, then it gets an attribute 'message_dict'.
If constructed with a string, or a list of strings, then it gets an attribute 'messages'.
The documentation on Model.clean() talks about ValidationError, but does not mention this at all, and instructs the user to look at messages_dict. [1]

If constructed with a dictionary of error messages, then the method update_error_dict(self, error_dict) will happily accept error_dict=None - it checks for and handles this condition. If constructed with a string or list, then not supplying error_dict is an error.

This sort of inconsistent behaviour is hard to explain. At the least, this behaviour should be documented clearly, but ideally the interface should be consistent, regardless of how it is constructed.

[1] http://docs.djangoproject.com/en/dev/ref/models/instances/#django.db.models.Model.clean

Change History (5)

comment:1 by Luke Plant, 13 years ago

Severity: Normal

comment:2 by Julien Phalip, 13 years ago

Triage Stage: UnreviewedDesign decision needed
Type: Cleanup/optimization

Just pointing out that cleaning up the interface is likely to break backwards compatibility, hence marking as DDN. I suggest that you write a patch with tests, so that we can see the extent to which this would break existing code.

comment:3 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:4 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:5 by Aymeric Augustin, 11 years ago

Resolution: needsinfo
Status: newclosed

This ticket describes a valid problem, but it doesn't suggest any concrete steps for improvement. Marking as 'needsinfo' because it isn't actionable.

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