Opened 10 years ago

Closed 10 years ago

#22719 closed New feature (wontfix)

Proposal: Delayed Form binding (with working example code)

Reported by: bernhard.hermann@… Owned by: nobody
Component: Forms Version: 1.6
Severity: Normal Keywords: bound unbound bind
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hello everybody!

I would like to propose adding a mechanism to "bind" Form-objects in a way other than at instantiation (e.g. Form(request.POST)).

This idea was inspired by the following assertion by Paul Graham:

"When I see patterns in my programs, I consider it a sign of trouble.
The shape of a program should reflect only the problem it needs to
solve. Any other regularity in the code is a sign, to me at least,
that I'm using abstractions that aren't powerful enough-- often that
I'm generating by hand the expansions of some macro that I need to
write." [ http://www.paulgraham.com/icad.html ]

Let me give a quick example from the Django documentation's "Working with forms":

def contact(request):
    if request.method == 'POST':
        form = ContactForm(request.POST)
        if form.is_valid():
            pass
        else:
            form = ContactForm()
    
    return render(request, 'contact.html', {'form': form,})

[ highlighted: http://pastebin.com/MAKUSrmY ]

This works well when I don't need to supply any keyword arguments. As soon as I want to do that, the code looks like this:

def contact(request):
    if request.method == 'POST':
        form = ContactForm(request.POST, prefix='something-%s' % (some_id,), label_suffix=': ')
        if form.is_valid():
            pass
    else:
        form = ContactForm(prefix='something-%s' % (some_id,), label_suffix=': ')
    
    return render(request, 'contact.html', {'form': form,})

[ highlighted: http://pastebin.com/9FKS9ExU ]

The really bad (IMHO: wrong) thing about this is that the code
prefix='something-%s' % (some_id,), label_suffix=': '
must be duplicated.

I am therefore using this class to bind the Form late:

class FlexiForm(forms.Form):
    def set_fields(self):
        for (f_name, attributes) in self._thefields.items():
            for (att_name, att_value) in attributes.items():
                setattr(self._theinstance.fields[f_name], att_name, att_value)

    def __init__(self, form, args=None, kwargs=None, fields=None):
        self._theform = form
        self._theargs = args or []
        self._thekwargs = kwargs or {}
        self._thefields = fields or {}
        self.bind()

    # !: make it possible to use kwargs here
    def bind(self, *args):
        self._theinstance = self._theform(*(list(args) + self._theargs), **self._thekwargs)
        self.set_fields()
        return self

    def __getattr__(self, name):
        return getattr(self._theinstance, name)

    def __setattr__(self, name, value):
        inst = self
        if name not in ['_theform', '_theargs', '_thekwargs', '_thefields', '_theinstance']:
            inst = self.__theinstance
        super(FlexiForm, inst).__setattr__(name, value)

[ highlighted: http://pastebin.com/wzY4qWUJ ]
(This is the actual verbatim class from my code. If you spot any errors, please tell!)

Melding this with the above example, we get:

def contact(request):
    form = FlexiForm(ContactForm, kwargs={'prefix': 'something-%s' % (some_id,), 'label_suffix': ': ',})
    if request.method == 'POST':
        form.bind(request.POST)  # substitutes internal ContactForm() with ContactForm(request.POST)
        if form.is_valid():
           pass
    else:
        pass  # Form already instantiated!

    return render(request, 'contact.html', {
        'form': form,
    })

[ highlighted: http://pastebin.com/zNA9MzRJ ]

Advantage: keyword (and other) arguments that are vitally important to many of my forms don't need to be duplicated in different locations.
Probability of mistakes/bugs is lowered, maintainability and readability improved.

I am sure there is a more elegant/correct way to do this.
I suspect it should be added to Django's Form class.

I am looking forward to your ...
[x] criticism
[x] reasons why this shouldn't be done
[x] promises that it will be implemented soon
[x] mindless ranting

best regards,
Bernhard

Change History (6)

comment:1 by Tim Graham, 10 years ago

I've used this pattern in the past:

form_kwargs = {'prefix': 'something-%s' % (some_id,), 'label_suffix': ': '}

form = MyForm(request.POST, **form_kwargs)
...
form = MyForm(**form_kwargs)

comment:2 by bernhard.hermann@…, 10 years ago

That's what came to my mind when I had finished this post ;-)
Unfortunately it has been a while since I wrote this "helper class" so I can't think of any case where your variant would be insufficient OTOH.

This could be the correct solution I was looking for when I wrote this "ugly" class.

comment:3 by Tim Graham, 10 years ago

Resolution: wontfix
Status: newclosed

Okay, will close this for now. Please reopen if you have any further thoughts on it.

comment:4 by bernhard.hermann@…, 10 years ago

Resolution: wontfix
Status: closednew

I've pondered over this for a bit longer now.
Conclusion: The duplication still bothers me a lot.

Your example

form_kwargs = {'prefix': 'something-%s' % (some_id,), 'label_suffix': ': '}
form = MyForm(request.POST, **form_kwargs)
form = MyForm(**form_kwargs)

would, IMO, be better written as

form = MyForm({'prefix': 'something-%s' % (some_id,), 'label_suffix': ': '})
form.bind(request.POST)

because this reduces code length, eliminates redundancy/repetition, is more concise and, to me at least, is clearer. In many cases it would be a wasted effort to introduce & duplicate the variable name 'form_kwargs' three times just to use the same arguments for the same form.
(note: the 'form' object in the first line of my example is equivalent to the 'form' object in the third line of your example.)

I am much in favor of the syntax I proposed.
I still think "binding" of Form objects should happen after instantiation. It's the only thing that differentiates a 'bound' form from an 'unbound' form and shouldn't introduce the necessity of copying all that code.

I would be happy to read your opinion(s) on this :-)

comment:5 by Tim Graham, 10 years ago

From a really quick glance at Form.__init__(), I cannot see any obvious technical issues that would prevent this from being implemented, but there may be issues in the details. Design decisions are best discussed on the django-developers mailing list through, so I'd recommend to post there.

comment:6 by anonymous, 10 years ago

Resolution: wontfix
Status: newclosed
form_kwargs = {'prefix': 'something-%s' % (some_id,), 'label_suffix': ': '}
form = MyForm(request.POST, **form_kwargs)
form = MyForm(**form_kwargs)

can be written as

form_kwargs = {'prefix': 'something-%s' % (some_id,), 'label_suffix': ': '}
form = MyForm(request.POST if request.method == 'post' else None, **form_kwargs)

and moved out of the if/else; I really don't see a good reason for bind

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