0

このコード例の一部をどこからリファクタリングするのか助けが必要なif (_obj is Application)ので、これは一般的なものになります。

public override void Body(object _obj, object _objInPreviousState)
        {

            if (_obj != null)
            {
                string Message = "";
                string Subject = "";
                if (_objInPreviousState == null)
                {
                    var emailParams = this.Param as Dictionary<string, string>;
                    if (emailParams != null)
                    {
                        Message = emailParams["Message"];
                        Subject = emailParams["Subject"];
                    }
                }
                var emails = userRepository().GetForRoles("RM").Select(c => c.Email);
                if (_obj is Application)
                {
                    var app = (Application)_obj;
                    var appInPreviousState = _objInPreviousState as Application;
                    if (appInPreviousState == null)
                    {
                        emailService().SendEmails("aps@somedomain.com", emails.ToArray(), Message, Subject);
                    }
                    else if (app.ApplicationStatus != appInPreviousState.ApplicationStatus)
                    {
                        emailService().SendEmails("aps@somedomain.com", emails.ToArray(), "Application: " + app.ID + " changed decision status: " + Enum.GetName(typeof(AppStatus), app.ApplicationStatus), "Check following application: " + app.ID);
                    }
                }
                else if (_obj is Product)
                {
                    var product = (Product)_obj;
                    var prodInPreviousState = _objInPreviousState as Product;
                    if (prodInPreviousState == null)
                    {
                        emailService().SendEmails("aps@somedomain.com", emails.ToArray(), Message, Subject);
                    }
                    else if (product.ProductStatusType != prodInPreviousState.ProductStatusType)
                    {
                        emailService().SendEmails("aps@somedomain.com", emails.ToArray(), "Product: " + product.ID + " for application " + product.ApplicationID + " changed decision status: " + Enum.GetName(typeof(AppStatus), product.ProductStatusType), "Check following application: " + product.ApplicationID);
                    }
                }

                else if (_obj is CES)
                {
                    var ces = (CES)_obj;
                    var cesInPreviousState = _objInPreviousState as CES;
                    if (cesInPreviousState == null)
                    {
                        emailService().SendEmails("aps@somedomain.com", emails.ToArray(), Message, Subject);
                    }
                    else if (ces.Status != cesInPreviousState.Status)
                    {
                        emailService().SendEmails("aps@somedomain.com", emails.ToArray(), "CES for application " + ces.ApplicationID + " changed decision status: " + Enum.GetName(typeof(CesStatuses), ces.Status), "Check following application: " + ces.ApplicationID);
                    }
                }
                else if (_obj is Comment)
                {
                    var comment = (Comment)_obj;
                    emailService().SendEmails("aps@somedomain.com", emails.ToArray(), "Comment for the following application: " + comment.ApplicationID + " with message: " + comment.Message + " on date: " + comment.CreatedDate, "Comment for the following application: " + comment.ApplicationID);
                }
                mLog.InfoLine("Sendet Email");
            }

        }
4

4 に答える 4

4

おそらくいくつかのインターフェイスを使用する必要があります。完全なコードは示していませんが、従うべきパターンを示しています。

interface IStatusItem
{
   void SendEmails(EmailService service);
}

public class Product : IStatusItem
{
   public void SendEmails(EmailService service)
   {
      // Send Email
   }
}

public class Application : IStatusItem
{
   public void SendEmails(EmailService service)
   {
      // Send Email
   }
}

次に、メイン コードにすべての if ブロックは必要ありません。IStatusItem のインスタンスの実装を呼び出すだけです。明らかに、そこに以前の状態を追加する必要があります。

override void Body(object _obj, object _objInPreviousState)
{
   IStatusItem sItem = obj as IStatusItem;
   if(sItem != null)
      sItem.SendEmails(emailService());
}
于 2012-04-25T13:44:37.550 に答える
1

簡単に改善できる2つのこと：

まず、ifネストを減らすためにいくつかのを反転します。特に：

if (_obj != null) { ... the entire function ... }

することができます

if (null == _obj) { return; }
... the rest ...

また、各if / elseボディを個別のメソッドに抽出します（ボディを選択して、メニューから[リファクタリング...メソッドの抽出]を選択するだけです。

最後に、これらすべてのメソッドを、さらにいくつかのパラメーターを受け取る単一のメソッドに一般化できるはずです。

于 2012-04-25T13:46:40.687 に答える
1
  1. _obj を逆変換if ( _obj == null ) return;
  2. ""宣言を次のように置き換えますstring.Empty
  3. string.Format を使用して、多くの連結を含む文字列をフォーマットします
  4. 電子メール アドレスを構成ファイルに抽出する

  5. アイテムのインターフェイスを作成する

    public interface IEmail{
  string GetMessage();
  string GetSubject();
}

  6. IEmailインスタンスを生成するファクトリを作成する

  7. 1 回の通話でメールを送信

     public void Body(object obj, object objInPreviousState)
    {
      const string Address= "aps@somedomain.com"; //extract to configuration
      IEmail item = GetEmailItem(_obj, _objInPreviousState);
      if(item != null) emailService().SendEmails( Address, emails.ToArray(), item.GetMessage(), item.GetSubject() );
    }
于 2012-04-25T14:17:29.747 に答える
1

この場合、オブジェクトを生成するファクトリを作成できます。

これは私がリファクタリングする方法です：

SomethingFactory は、AbstractSomething 派生クラス (ConcreteSomethingA、ConcreteSomethingB など) を生成します。Factory は、"_obj" に応じて ConcreteSomethings を生成し、

public override void Body(object _obj, object _objInPreviousState)

具体的なクラスで実装されるため、システムを簡単に拡張できます

于 2012-04-25T13:49:47.540 に答える