Code

#19668 closed New feature (wontfix)

Form enhancement: `Form.set_data` to set data and files

Reported by: bruth Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

This simply moves the logic for setting is_bound, data and files into a method which enables setting the data after form initialization. This is a backwards compatible. This makes it easier to not have to initialize a form twice in view. It changes from this idiom:

def view(request):
    if request.method == 'POST':
        form = Form(request.POST, request.FILES)
        ...
    else:
        form = Form()
    ...

to simply this:

def view(request):
    form = Form()
    if request.method == 'POST':
        form.set_data(request.POST, request.FILES)
    ...

This reduces clutter and redundancy when there are a lot of form arguments passed.

Attachments (0)

Change History (6)

comment:1 Changed 15 months ago by bruth

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 15 months ago by bruth

  • Summary changed from Form enhancement: `Form.set_data` to set data an files to Form enhancement: `Form.set_data` to set data and files

comment:3 Changed 15 months ago by anonymous

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Design decision needed

I'm -0 on this change, but i'm not a core dev.

The argument against this would be that much of a form's functionality changes based on data, fields, valid/invalid, cleaned data, errors etc.. Changing the data at the wrong time could cause a lot of problems. Changing set_data to raise an error if the form is already bound may be enough to address some of those concerns. I'd suggest raising the issue on django-developers to get some opinions from core devs.

comment:4 Changed 15 months ago by anonymous coward

-1

You don't save a single line of code (unless you copy the provided example, which doesn't complete the antithesis version), are adding another method to maintain, and are creating the potential for internal problems (as mentioned above), not to mention making Form less extensible.

comment:5 Changed 15 months ago by rafales

-1, and you can make it even shorter:

form = Form(request.POST or None, request.FILES or None)
if request.method == 'POST':
    # code

comment:6 Changed 15 months ago by claudep

  • Resolution set to wontfix
  • Status changed from new to closed

This proposal received mostly negative opinions until now (also on django-developers). I also think that this would be an important change in the contract that Form data are set at initialization time, for few benefits but a high risk of triggering bugs in the form stack.
Anyway, thanks for taking the time to make the proposal.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.