3
\$\begingroup\$

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:

  1. 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?
  2. 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.
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 31, 2016 at 20:35
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

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 <= you can just do <:

if set(request.POST) < required:

as it reads:

You have less than you need for the required fields. (削除ここまで)

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.

answered Jul 31, 2016 at 23:11
\$\endgroup\$
1
  • \$\begingroup\$ Your first and last points especially were helpful. However, I have to disagree with you on your if set(request.POST) < required: statement. request.POST may contain items which are not in required but I want to check whether or not everything in required is also in request.POST, i.e. whether or not required is a subset of request.POST. An example of something in request.POST but not in required would be the address_2 field, which is not required. \$\endgroup\$ Commented Aug 1, 2016 at 17:21

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.