Comment puis-je améliorer ce point de vue « enregistrer » dans Django?

Question

J'ai un site basé sur Django qui permet aux utilisateurs d'enregistrer (mais nécessite un administrateur d'approuver le compte avant de pouvoir voir certaines parties du site). Je me base hors de django.contrib.auth. J'obliger les utilisateurs à enregistrer une adresse e-mail d'un certain nom de domaine, donc j'ai surchargé et UserCreationForm méthodes de save() du clean_email().

Ma page d'inscription utilise la vue suivante. Je suis intéressé à entendre parler de la façon dont je pourrais améliorer cette amélioration vue de code ou l'amélioration des processus (ou toute autre chose, vraiment).

def register(request):
    if request.method == 'POST':
        form = UserCreationForm(request.POST)

        if form.is_valid():
            message = None

            form.save()

            username = form.cleaned_data['username']
            password = form.cleaned_data['password1']

            user = authenticate(username=username, password=password)

            first_name = form.cleaned_data['first_name']
            last_name = form.cleaned_data['last_name']
            email = user.email

            # If valid new user account
            if (user is not None) and (user.is_active):
                login(request, user)
                message = "<strong>Congratulations!</strong> You have been registered."

                # Send emails
                try:
                    # Admin email
                    pk = None
                    try: pk = User.objects.filter(username=username)[0].pk
                    except: pass

                    admin_email_template = loader.get_template('accounts/email_notify_admin_of_registration.txt')
                    admin_email_context = Context({
                        'first_name': first_name,
                        'last_name': last_name,
                        'username': username,
                        'email': email,
                        'pk': pk,
                    })
                    admin_email_body = admin_email_template.render(admin_email_context)
                    mail_admins("New User Registration", admin_email_body)

                    # User email
                    user_email_template = loader.get_template('accounts/email_registration_success.txt')
                    user_email_context = Context({
                        'first_name': form.cleaned_data['first_name'],
                        'username': username,
                        'password': password,
                    })
                    user_email_body = user_email_template.render(user_email_context)
                    user.email_user("Successfully Registered at example.com", user_email_body)
                except:
                    message = "There was an error sending you the confirmation email. You should still be able to login normally."
            else:
                message = "There was an error automatically logging you in. Try <a href=\"/login/\">logging in</a> manually."

            # Display success page
            return render_to_response('accounts/register_success.html', {
                    'username': username,
                    'message': message,
                },
                context_instance=RequestContext(request)
            )
    else: # If not POST
        form = UserCreationForm()

    return render_to_response('accounts/register.html', {
            'form': form,
        },
        context_instance=RequestContext(request)
    )

Answer 1

Vous ne même pas besoin de ce code, mais je pense que le style:

pk = None
try: pk = User.objects.filter(username=username)[0].pk
except: pass

est plus naturellement écrit comme:

try:
    user = User.objects.get(username=username)
except User.DoesNotExist:
    user = None

puis dans votre admin notifier l'utilisation du modèle au lieu de {{ user.id }} {{ pk }}.

Mais, comme je l'ai dit, vous n'avez pas besoin que le code du tout parce que vous avez déjà un objet utilisateur de votre appel à authenticate().

En général en Python, il est considéré comme une mauvaise pratique d'avoir le gestionnaire d'exception dans un try / except vide. En d'autres termes, toujours saisir une exception spécifique, comme dans ce cas User.DoesNotExist.

Il est aussi mauvaise pratique d'avoir plusieurs lignes à l'intérieur de la partie du try try / except. Il est préférable de coder sous forme de cette façon:

try:
    ... a line of code that can generate exceptions to be handled ...
except SomeException:
    ... handle this particular exception ...
else:
    ... the rest of the code to execute if there were no exceptions ...

Une recommandation finale, mineur, est de ne pas envoyer l'e-mail directement à votre avis, car ce ne sera pas l'échelle si votre site commence à voir les demandes d'inscription lourdes. Il est préférable d'ajouter dans l'application django-mailer pour décharger le travail dans une file d'attente gérée par un autre processus.

Answer 2

Personnellement, j'essaie de mettre la branche courte d'une première instruction if-else. Surtout si elle retourne. Ceci afin d'éviter d'obtenir de grandes branches où il est difficile de voir ce qui se termine là où. Beaucoup d'autres font comme vous l'avez fait et de mettre un commentaire à la déclaration d'autre. Mais ne marche python ont toujours un fin de l'instruction de bloc - comme si un formulaire est pas valable pour vous.

Ainsi, par exemple:

def register(request):
    if request.method != 'POST':
       form = UserCreationForm()
       return render_to_response('accounts/register.html', 
                                 { 'form': form, },
                                 context_instance=RequestContext(request)
                                 )

    form = UserCreationForm(request.POST)
    if not form.is_valid():
       return render_to_response('accounts/register.html', 
                                 { 'form': form, },
                                 context_instance=RequestContext(request)
                                 )
    ...

Answer 3

Première réponse: Il a l'air un diable de beaucoup mieux que 95% des questions "améliorer mon code"

.

Y at-il quelque chose en particulier que vous n'êtes pas satisfait?