HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythondjangoMinor

Profile creation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
creationprofilestackoverflow

Problem

I was creating a view with Django that would create a user profile for a website and this is what I came up with:

if (request.POST):
    required = set( ['username', 'email', 'first_name', 'last_name', 'password', 
        'address_1', 'city', 'state', 'zip_code'] )
    if (not (required <= set(request.POST))):
        return render( request, 'profiles/create.html', { 'missing' : required - set(request.POST) } )
    username = request.POST['username']
    email = request.POST['email']
    if (not Profile.is_unique(email)):
        return render( request, 'profiles/create.html', {'message' : 'Email address already exists!'} )
    first_name = request.POST['first_name']
    last_name = request.POST['last_name']
    password = request.POST['password']
    address_1 = request.POST['address_1']
    address_2 = request.POST['address_2'] if 'address_2' in request.POST else None
    city = request.POST['city']
    state = request.POST['state']
    zip_code = request.POST['zip_code']
    new_id = Create(username, email, first_name, last_name, password, address_1,
        city, state, zip_code, address_2)
    return redirect('profiles.views.Details', profile_id = new_id)
else:
    return render(request, 'profiles/create.html')


I am about 65% happy with this code as it is, especially the use of sets. However, there are some areas I would like to improve on:

  • Is there any way I could avoid assigning to individual variables from the request.POST calls, maybe using setattr? Would that be faster or slower than doing it this way?



  • I have a large if at the top which checks whether or not the page has been submitted already to avoid error messages popping up the first time somebody navigates to the page but I'm not sure if this is the best way to go about it.

Solution

Instead of doing:

required = set( ['username', 'email', 'first_name', 'last_name', 'password', 
    'address_1', 'city', 'state', 'zip_code'] )


You can just use {}:

required = {'username', 'email', 'first_name', 'last_name', 'password', 
    'address_1', 'city', 'state', 'zip_code'}


You don't need a lot of parenthesis around your if statements, so:

if (request.POST):


becomes:

if request.POST:


In the if statement:

if (not (required <= set(request.POST))):


You can first get rid of the outer parenthesis:

if not (required <= set(request.POST)):


Instead of saying not

I would not make so many trivial variables such as:

username = request.POST['username']


Just use
request.POST['username']. You don't use username, email ... that often, I would just use request.POST['thing']` to save space. Although there are many reasons not to make code as terse as possible, adding lots of these variable declarations makes your code hard to read.

Code Snippets

required = set( ['username', 'email', 'first_name', 'last_name', 'password', 
    'address_1', 'city', 'state', 'zip_code'] )
required = {'username', 'email', 'first_name', 'last_name', 'password', 
    'address_1', 'city', 'state', 'zip_code'}
if (request.POST):
if request.POST:
if (not (required <= set(request.POST))):

Context

StackExchange Code Review Q#136484, answer score: 2

Revisions (0)

No revisions yet.