6
\$\begingroup\$

I have 4 user account types, realtor, insurance broker, auto insurance agent, and mortgage broker. Each one of the users will have a different form for each to submit. I have the following in views.py to render the form:

def _get_form(profile, postdata=None):
    # @TODO - come up with a better solution to render forms on user types than this.
    form = None

    if profile.affiliate_type == UserProfile.INSURANCE_BROKER:
        if postdata:
            form = InsuranceBrokerForm(postdata, instance=profile)
        else:
            form = InsuranceBrokerForm(instance=profile)
    elif profile.affiliate_type == UserProfile.AUTO_DEALERSHIP:
        if postdata:
            form = AutoDealershipForm(postdata, instance=profile)
        else:
            form = AutoDealershipForm(instance=profile)
        form = AutoDealershipForm
    elif profile.affiliate_type == UserProfile.MORTGAGE_BROKER:
        if postdata:
            form = MortgageBrokerForm(postdata, instance=profile)
        else:
            form = MortgageBrokerForm(instance=profile)
    elif profile.affiliate_type == UserProfile.REALTOR:
        if postdata:
            form = RealtorForm(postdata, instance=profile)
        else:
            form = RealtorForm(instance=profile)
    return form

and it's being used like so:

@login_required
def affiliate_form(request, template_name="affiliates/affiliate_form.html"):
    user = request.user
    profile = get_object_or_404(UserProfile, user=user)
    form = _get_form(profile)
    if profile.user != user:
        return HttpResponseForbidden("Forbidden. Please try again.")
    if request.method == 'POST':
        postdata = request.POST.copy()
        form = _get_form(postdata)
        if form.is_valid():
            form.save()
            messages.success(request, 'Your lead has been created.')
            url = urlresolvers.reverse('affiliates')
            return redirect(url)
    else:
        form = _get_form(profile)

    return render(request, template_name, {
        'form': form
    })

As you can see, I am calling _get_form(profile) but this seems dirty. Is there a cleaner way to go about this?

\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

Dicts are good for lookups

When encountering stacked ifs that are all alike, I tend to pickup a dict and try to put it to good use. One way to structure the variable parts of your routine is with something like:

user_form_types = {
    UserProfile.INSURANCE_BROKER: InsuranceBrokerForm,
    UserProfile.AUTO_DEALERSHIP: AutoDealershipForm,
    UserProfile.MORTGAGE_BROKER: MortgageBrokerForm,
    UserProfile.REALTOR: RealtorForm,
}

This one structure allows the variability in the routine to be described in a manner that makes it easy to read and understand that variability, and easy to use the resulting data.

Using data as code

Since Python makes it easy to reference a class and then instantiate it, the dict can be used, and the routine can be boiled down to simply:

user_form_type = user_form_types.get(profile.affiliate_type)
if not user_form_type:
    form = None
else:
    if postdata:
        form = user_form_type(postdata, instance=profile)
    else:
        form = user_form_type(instance=profile)
return form

Entire Recast Routine

def _get_form(profile, postdata=None):

    user_types = {
        UserProfile.INSURANCE_BROKER: InsuranceBrokerForm,
        UserProfile.AUTO_DEALERSHIP: AutoDealershipForm,
        UserProfile.MORTGAGE_BROKER: MortgageBrokerForm,
        UserProfile.REALTOR: RealtorForm,
    }

    user_form_type = user_types.get(profile.affiliate_type)
    if not user_form_type:
        form = None
    else:
        if postdata:
            form = user_form_type(postdata, instance=profile)
        else:
            form = user_form_type(instance=profile)
    return form
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.