4

Webサイトのパスワードリセット機能を作成しています。パスワードリセットの最初のステップを実装する必要があります。

  1. ユーザーは自分のメールアドレスをパスワードリセットフォームに入力します。
  2. 電子メールを持つユーザーが登録されているかどうかをシステムがチェックします。
  3. ユーザーが見つかった場合、システムはパスワードリセットURLとuniqeトークンを含む電子メールを送信します。
  4. ユーザーが見つからない場合、システムはこの電子メールに対してパスワードのリセットが開始されたことを通知する電子メールを送信しますが、ユーザーアカウントは存在しません。

publicメソッド--RequestPasswordResetを実装するサービスクラスがあり、このメソッドは非常に手続き型です。

public void RequestPasswordReset(string email)
{
    if(!IsValidEmail(email))
    {
        throw new ArgumentException("email");
    }

    var user = this.repository.FindByEmail(email);
    if(user != null)
    {
        user.PasswordResetToken.Set(this.tokenGenerator.NewToken());
        this.emailService.Send(this.from, 
                               user.Email, 
                               "Password reset", 
                               "Your reset url: http://mysite.com/?t=" + 
                                     user.PasswordResetToken.Value);
    }
    else
    {
    this.emailService.Send(this.from,
                        user.Email, 
                        "Requested password reset", 
                        "Someone requested password reset at http://mysite.com");
    }
}

このメソッドは、単一責任の原則に違反します。ユーザーの存在を確認し、ユーザートークンをリセットし、電子メールを送信します。

このようなソリューションの主な問題は、追加のステップを追加する必要がある場合、それらの実装をRequestPasswordResetメソッドに追加する必要があり、メソッドがますます複雑になることです。追加の手順として、たとえば、ユーザーが別の関連システムに既に登録されているかどうかを確認し、システムに新しいアカウントを作成するか、ユーザーアカウントを作成するようにアドバイスすることができます。

私はコマンドパターンを見ていました-サービスクラスを別々のコマンドにリファクタリングするのは良いかもしれません、そしてRequestPasswordResetはそれらのコマンドの1つである可能性があります。ただし、RequestPasswordResetメソッド内のステップに関する主な問題は解決されません。

また、Chain of Responsibilityパターンも調べていました。ステップを順番に処理するのがよいのですが、制御フローを処理するためにどのように使用できるのかわかりません。さまざまな条件を実装する必要があります。また、各ハンドラーが同様のアクションを実行する必要があるように見え、制御フロー全体がどのように変化するかは明確ではありません。

そのような手続き型コードをリファクタリングするための最良のアプローチは何でしょうか？

4

3 に答える 3

6

この方法は単一責任の原則に違反します

実はそうは思いません。このサービスの「単一責任」はユーザーのパスワードをリセットすることであり、他のオブジェクト（ユーザーリポジトリとメールサービス）と調整することでこれを非常にうまく行います。パスワードをリセットするプロセスが変更されると、このクラスも変更され、問題はありません。

于 2012-08-16T06:46:14.897 に答える
3

I think your code is fine as it is. If you absolutely want to refactor something, you might split the code up a little more just to simplify readability of the main method; something like this: 

public void RequestPasswordReset(string email)
{
    ValidateEmail(email); // May throw exception if invalid

    var user = this.repository.FindByEmail(email);
    if(user != null)
    {
        GenerateTokenAndSendPasswordResetEmail(user);
    }
    else
    {
        SendPasswordResetEmail(email);
    }
}

Other than that, I would leave it as it is. Your problem is really quite simple, so there is no reason to look for a complex solution! :)

于 2012-08-16T07:16:48.407 に答える
2

このコードを使用する最善の方法は、完全にテストされていることを確認することです。すべてのコード パスがテストされていることを確認します。

そして、それを乗り越えてください！

すべてのコードが設計パターン、単一責任の原則などに従う必要があるという考えを克服してください。これらのパターンは、実際のコードを調べることで発見されました。

ここに作業コードがあります。それを乗り越えて、次の仕事に取り掛かります。

于 2012-08-16T06:51:41.767 に答える