Djangoでこの「登録」ビューを改善するにはどうすればよいですか？

Question

私はDjangoベースのサイトを持っています。これにより、ユーザーは登録できますが、サイトの特定の部分を表示するには管理者がアカウントを承認する必要があります。私はdjango.contrib.authの外にそれを置いています。私はユーザーに特定のドメイン名の電子メールアドレスを登録する必要があるので、UserCreationFormのsave()とclean_email()メソッドをオーバーライドしました。

私の登録ページは以下のビューを使用しています。私はこのビューコードの改善やプロセスの改善（または他の何か、本当に）をどのように改善するかについて聞いてみたいと思います。

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

あなたも、このコードは必要ありませんが、私はスタイルを考える：

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

はより自然のように書かれている：

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

、その後、あなたの管理者にテンプレートの使用{{ user.id }}の代わりに通知{{ pk }}。

しかし、私が言ったように、authenticate()への呼び出しのユーザーオブジェクトを既に持っているので、そのコードはまったく必要ありません。

一般に、Pythonではtry/exceptブロック内の例外ハンドラを空にすることは貧弱な方法です。つまり、この場合は常にUser.DoesNotExistなどの特定の例外をキャプチャします。

try/exceptブロックのtry部分に多数の行があることも貧弱です。最終的に、マイナー、勧告はあなたのサイトが重い登録要求を確認するために開始した場合、これはスケールしませんので、あなたのビューで直接電子メールを送信しないことです

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 ... 

：この方法をコーディングするためのより良い形です。 django-mailerアプリを追加して、別のプロセスが処理するキューに作業をオフロードする方がよいでしょう。

Answer 2

最初の応答：「自分のコードを改善する」質問の95％よりずっとよく見えます。

あなたには特に不満がありますか？

Answer 3

私は個人的に最初にif-else文の短い枝を入れてみます。特に返ったら。これはどこで終わるのか分かりにくい場所で大きな枝を得るのを避けるためです。他の多くの人があなたがやったように、else文にコメントを付けます。しかし、Pythonはブロック文の終わりを常に持っていません - フォームがあなたにとって有効でない場合のように。例えばので

：私はに不満だ

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) 
           ) 
    ...