This code comes straight out from the example code in a Django book. I find it quite bad, mainly because the use of flags (if ajax
then again if ajax
) and unnecessarily bigger variable scope (set title=''
, tags=''
and then use these variables in different flows). The function body also seems way too long.
I guess I'll subtract the book title (for the record I find the book in general good).
How would you rate this code, say out of 10 (10=very good, 3 and below being unacceptable)? (I'd rate it a 3)
The reason I ask is, I had rejected similar code in the past during code review and now I'm wondering whether I've been too strict or didn't understand python culture (I'm new to python).
@login_required
def bookmark_save_page(request):
ajax = 'ajax' in request.GET
if request.method == 'POST':
form = BookmarkSaveForm(request.POST)
if form.is_valid():
bookmark = _bookmark_save(request, form)
if ajax:
variables = RequestContext(request, {
'bookmarks': [bookmark],
'show_edit': True,
'show_tags': True
})
return render_to_response(
'bookmark_list.html', variables
)
else:
return HttpResponseRedirect(
'/user/%s/' % request.user.username
)
else:
if ajax:
return HttpResponse(u'failure')
elif 'url' in request.GET:
url = request.GET['url']
title = ''
tags = ''
try:
link = Link.objects.get(url=url)
bookmark = Bookmark.objects.get(
link=link,
user=request.user
)
title = bookmark.title
tags = ' '.join(
tag.name for tag in bookmark.tag_set.all()
)
except (Link.DoesNotExist, Bookmark.DoesNotExist):
pass
form = BookmarkSaveForm({
'url': url,
'title': title,
'tags': tags
})
else:
form = BookmarkSaveForm()
variables = RequestContext(request, {
'form': form
})
if ajax:
return render_to_response(
'bookmark_save_form.html',
variables
)
else:
return render_to_response(
'bookmark_save.html',
variables
)