优化Python/Django视图代码
我刚开始接触Python/Django和编程。由于我掌握的工具有限,我写了三个视图函数,用于用户注册后:这让用户可以在激活账户之前添加信息和上传缩略图。
我把目前写的代码贴出来,希望能有经验更丰富的人帮我看看怎么改进。毫无疑问,这些代码很粗糙,明显是新手写的,但我觉得通过写代码来学习是最有效的——看到改进的地方,学习新工具,然后再重写。
我知道回答这个问题需要花费不少时间。因此,我会为这个问题提供200分的悬赏。 StackOverflow只允许我在问题发布两天后添加悬赏,所以我会在周二(一旦可以添加时)为这个问题添加悬赏。请注意,由于我在添加悬赏之前不会选择答案,因此在悬赏添加之前提供的答案仍然会被视为“好像”这个问题有悬赏。
以下是我自我注释的代码。特别是,我在每个函数的前10到14行有很多重复代码,用来根据用户是否登录、是否已经填写过这些信息、是否有会话信息等来重定向用户。
# in model.py
choices = ([(x,str(x)) for x in range(1970,2015)])
choices.reverse()
class UserProfile(models.Model):
"""
Fields are user, network, location, graduation, headline, and position.
user is a ForeignKey, unique = True (OneToOne). network is a ForeignKey.
loation, graduation, headline, and position are optional.
"""
user = models.ForeignKey(User, unique=True)
network = models.ForeignKey(Network)
location = models.CharField(max_length=100, blank=True)
graduation = models.CharField(max_length=100, blank=True, choices=choices)
headline = models.CharField(max_length=100, blank=True)
positions = models.ManyToManyField(Position, blank=True)
avatar = models.ImageField(upload_to='images/%Y/%m/%d', blank=True, default='default_profile_picture.jpg')
# if the user has already filled out the 'getting started info', set boolean=True
getting_started_boolean = models.BooleanField(default=False)
一般背景: 用户注册后,我给他们两个会话变量:
request.session['location'] = get_location_function(request)
request.session['username'] = new_user # this is an email address
用户注册后,他们会被重定向到入门页面。
第一页:
# in views.py
def getting_started_info(request, positions=[]):
"""
This is the first of two pages for the user to
add additional info after they have registrered.
There is no auto log-in after the user registers,
so the individiaul is an 'inactive user' until he
clicks the activation link in his email.
"""
location = request.session.get('location')
if request.user.is_authenticated():
username = request.user.username # first see if the user is logged in
user = User.objects.get(email=username) # if so, get the user object
if user.get_profile().getting_started_boolean:
return redirect('/home/') # redirect to User home if user has already filled out page
else:
pass
else:
username = request.session.get('username', False) # if not logged in, see if session info exists from registration
if not username:
return redirect('/account/login') # if no session info, redirect to login page
else:
user = User.objects.get(email=username)
if request.method == 'POST':
if 'Next Step' in request.POST.values(): # do custom processing on this form
profile = UserProfile.objects.get(user=user)
profile.location = request.POST.get('location')
populate_positions = []
for position in positions:
populate_positions.append(Position.objects.get(label=position))
profile.positions = request.POST.get('position')
profile.headline = request.POST.get('headline')
profile.graduation = request.POST.get('graduation')
profile.save()
return redirect('/account/gettingstarted/add_pic/')
else:
form = GettingStartedForm(initial={'location': location})
return render_to_response('registration/getting_started_info1.html', {'form':form, 'positions': positions,}, context_instance=RequestContext(request))
第二页:
def getting_started_pic(request):
"""
Second page of user entering info before first login.
This is where a user uploads a photo.
After this page has been finished, set getting_started_boolean = True,
so user will be redirected if hits this page again.
"""
if request.user.is_authenticated():
username = request.user.username
user = User.objects.get(email=username)
if user.get_profile().getting_started_boolean:
return redirect('/home/')
else:
pass
else:
username = request.session.get('username', False)
if not username:
return redirect('/account/login')
else:
user = User.objects.get(email=username)
try:
profile = UserProfile.objects.get(user=User.objects.get(email=username)) # get the profile to display the user's picture
except UserProfile.DoesNotExist: # if no profile exists, redirect to login
return redirect('/account/login') # this is a repetition of "return redirect('/account/login/')" above
if request.method == 'POST':
if 'upload' in request.POST.keys():
form = ProfilePictureForm(request.POST, request.FILES, instance = profile)
if form.is_valid():
if UserProfile.objects.get(user=user).avatar != 'default_profile_picture.jpg': # if the user has an old avatar image
UserProfile.objects.get(user=user).avatar.delete() # delete the image file unless it is the default image
object = form.save(commit=False)
try:
t = handle_uploaded_image(request.FILES['avatar']) # do processing on the image to make a thumbnail
object.avatar.save(t[0],t[1])
except KeyError:
object.save()
return render_to_response('registration/getting_started_pic.html', {'form': form, 'profile': profile,}, context_instance=RequestContext(request))
if 'finish' in request.POST.keys():
UserProfile.objects.filter(user=user).update(getting_started_boolean='True') # now add boolean = True so the user won't hit this page again
return redirect('/account/gettingstarted/check_email/')
else:
form = ProfilePictureForm()
return render_to_response('registration/getting_started_pic.html', {'form': form, 'profile': profile,}, context_instance=RequestContext(request))
第三页:
def check_email(request):
"""
End of getting started. Will redirect to user home
if activation link has been clicked. Otherwise, will
allow user to have activation link re-sent.
"""
if request.user.is_authenticated(): # if the user has already clicked his activation link, redirect to User home
return redirect('/home/')
else: # if the user is not logged in, load this page
resend_msg=''
user = email = request.session.get('username')
if not email:
return redirect('/account/login/')
if Site._meta.installed:
site = Site.objects.get_current()
else:
site = RequestSite(request)
if request.method == 'POST':
RegistrationProfile.objects.resend_activation(email, site)
resend_msg = 'An activation email has been resent to %s' %(email)
return render_to_response('registration/getting_started_check_email.html', {'email':email, 'resend_msg':resend_msg}, context_instance=RequestContext(request))
return render_to_response('registration/getting_started_check_email.html', {'email':email, 'resend_msg':resend_msg}, context_instance=RequestContext(request))
5 个回答
(我还没有看完你的代码,但我给你一些建议,让代码看起来更好)
我认为django-annoying的render_to装饰器可以让代码更容易阅读和编写。(重定向依然有效):
@render_to('registration/getting_started_info1.html')
def getting_started_info(request, positions=[]):
...
return {'form':form, 'positions': positions}
@render_to('registration/getting_started_pic.html')
def getting_started_pic(request):
...
return {'form': form, 'profile': profile}
@render_to('registration/getting_started_check_email.html')
def check_email(request):
...
return {'email':email, 'resend_msg':resend_msg}
我会有一些不同的做法:
在你的 models.py 文件中,你为
graduation
字段使用了choices
参数,但没有在模型中定义这些选项,而且没有使用全大写字母,通常在 Python 中全大写字母用来表示常量。这样做会更好:class UserProfile(models.Model): ... CHOICES = [('choice1', 'Choice 1'), ('choice2', 'Choice2')] graduation = models.CharField(max_length=100, blank=True, choices=CHOICES)
你在
avatar
字段上使用了可选的default
,其实用blank=True
会更合理,这样做会让你后面的逻辑更简单。在你的视图中:if UserProfile.objects.get(user=user).avatar != 'default_profile_picture.jpg': UserProfile.objects.get(user=user).avatar.delete()
与其这样,不如在你的模板中直接处理没有头像的情况。
在你的
getting_started_info
视图中,positions
的默认参数是一个可变对象,positions=[]
。这让我有点不舒服,虽然在你的情况下不会出问题,因为它从未被修改。一般来说,最好避免在 Python 中使用可变对象作为默认参数,因为函数定义只会被评估一次。最好避免这样做。>>> def foo(l=[]): ...: l.append(1) ...: return l ...: >>> foo() <<< [1] >>> foo() <<< [1, 1] >>> foo() <<< [1, 1, 1]
你写了
else: pass
,这其实是多余的,完全可以去掉这个 `else`。if user.get_profile().getting_started_boolean: return redirect('/home/') # else: # pass
如果有机会的话,我会再提一些我对你代码的其他看法。
我最开始尝试用django.contrib.formtools.wizard来模拟你的注册流程,但发现这太复杂了,毕竟你的流程只有两个步骤,其中一个只是选择一张图片。如果你打算保持多步骤的注册流程,我强烈建议你考虑使用表单向导解决方案。这样可以让系统自动处理请求之间的状态,你只需要定义一系列的表单就可以了。
不过,我决定把整个流程简化成一个步骤。通过使用一个基本的模型表单,我们可以在一页上轻松捕获所有需要的用户资料信息,而且代码量非常少。
我还选择了基于类的视图,这是在Django 1.3中引入的。这种方式让一些重复的代码(比如在每个函数顶部检查当前流程的代码)更容易管理,虽然一开始可能会显得复杂一些。但一旦你理解了它们,它们在很多情况下都非常好用。好了,接下来是代码。
# in models.py
graduation_choices = ([(x,str(x)) for x in range(1970,2015)])
graduation_choices.reverse()
class UserProfile(models.Model):
# usually you want null=True if blank=True. blank allows empty forms in admin, but will
# get a database error when trying to save the instance, because null is not allowed
user = models.OneToOneField(User) # OneToOneField is more explicit
network = models.ForeignKey(Network)
location = models.CharField(max_length=100, blank=True, null=True)
graduation = models.CharField(max_length=100, blank=True, null=True, choices=graduation_choices)
headline = models.CharField(max_length=100, blank=True, null=True)
positions = models.ManyToManyField(Position, blank=True)
avatar = models.ImageField(upload_to='images/%Y/%m/%d', blank=True, null=True)
def get_avatar_path(self):
if self.avatar is None:
return 'images/default_profile_picture.jpg'
return self.avatar.name
def is_complete(self):
""" Determine if getting started is complete without requiring a field. Change this method appropriately """
if self.location is None and self.graduation is None and self.headline is None:
return False
return True
我借用了这个回答中的一部分来处理默认图片位置,因为这个建议非常好。把“渲染哪张图片”交给模板和模型来处理。另外,在模型上定义一个方法来回答“是否完成?”的问题,而不是再定义一个字段,这样可以简化流程。
# forms.py
class UserProfileForm(forms.ModelForm):
class Meta:
model = UserProfile
widgets = {
'user': forms.HiddenInput() # initial data MUST be used to assign this
}
这是一个基于UserProfile对象的简单ModelForm。这将确保模型的所有字段都能在表单中显示,并且所有内容可以原子性地保存。这是我主要偏离你方法的地方。与其使用多个表单,不如用一个就可以。我觉得这样用户体验更好,特别是字段并不多的情况下。你还可以在用户想修改信息时重用这个表单。
# in views.py - using class based views available from django 1.3 onward
class SignupMixin(View):
""" If included within another view, will validate the user has completed
the getting started page, and redirects to the profile page if incomplete
"""
def dispatch(self, request, *args, **kwargs):
user = request.user
if user.is_authenticated() and not user.get_profile().is_complete()
return HttpResponseRedirect('/profile/')
return super(SignupMixin, self).dispatch(request, *args, **kwargs)
class CheckEmailMixin(View):
""" If included within another view, will validate the user is active,
and will redirect to the re-send confirmation email URL if not.
"""
def dispatch(self, request, *args, **kwargs):
user = request.user
if user.is_authenticated() and not user.is_active
return HttpResponseRedirect('/confirm/')
return super(CheckEmailMixin, self).dispatch(request, *args, **kwargs)
class UserProfileFormView(FormView, ModelFormMixin):
""" Responsible for displaying and validating that the form was
saved successfully. Notice that it sets the User automatically within the form """
form_class = UserProfileForm
template_name = 'registration/profile.html' # whatever your template is...
success_url = '/home/'
def get_initial(self):
return { 'user': self.request.user }
class HomeView(TemplateView, SignupMixin, CheckEmailMixin):
""" Simply displays a template, but will redirect to /profile/ or /confirm/
if the user hasn't completed their profile or confirmed their address """
template_name = 'home/index.html'
这些视图可能是最复杂的部分,但我觉得比一堆复杂的视图函数代码要容易理解多了。我在函数内部做了简单的注释,这样应该能稍微帮助理解。剩下的就是把你的URL连接到这些视图类上。
# urls.py
urlpatterns = patterns('',
url(r'^home/$', HomeView.as_view(), name='home'),
url(r'^profile/$', UserProfileFormView.as_view(), name='profile'),
url(r'^confirm/$', HomeView.as_view(template_name='checkemail.html'), name='checkemail'),
)
现在这些代码都是未经测试的,所以可能需要调整才能正常工作,并且要与你的网站集成。此外,这完全不同于你的多步骤流程。如果字段很多,多步骤流程会很好,但单独为头像设置一个页面似乎有点过了。希望无论你选择哪种方式,这些建议都有帮助。
关于基于类的视图的一些链接:
我还想提一下你代码中的一些问题。例如,你有这个:
populate_positions = []
for position in positions:
populate_positions.append(Position.objects.get(label=position))
可以用这个替换:
populate_positions = Position.objects.filter(label__in=positions)
前者会对每个位置都查询数据库,而后者在评估时只会进行一次查询。
还有;
if request.user.is_authenticated():
username = request.user.username
user = User.objects.get(email=username)
上面的代码是多余的。你已经可以访问用户对象了,然后又试图再次获取它。
user = request.user
完成。
顺便说一下,如果你想用电子邮件地址作为用户名,你会遇到问题。数据库最多只接受30个字符(这是contrib.auth中User模型的限制)。可以看看这个讨论串中的一些评论,讨论了一些潜在的问题。