ブールが多すぎるメソッドをリファクタリングする

Question

私はこのメソッドをC＃で実装していますが、リファクタリングしたいと考えています。ちょうどブールとラインが多すぎます。何が最良のリファクタリングでしょうか。新しいクラスを作るのはちょっと残酷すぎるようですが、2人で簡単に切断するのは難しいようです。任意の洞察力またはポインタが評価されます。そのまま実際に、私は個人的にそれを残すだろう

private DialogResult CheckForSireRestrictionInSubGroup(bool deletingGroup,string currentId) 
    { 
     DialogResult result = DialogResult.No; 
     if (!searchAllSireList) 
     { 
      DataAccessDialog dlg = BeginWaitMessage(); 
      bool isClose = false; 
      try 
      { 
       ArrayList deletedSire = new ArrayList(); 
       ISireGroupBE sireGroupBE = sireController.FindSireGroupSearch(); 

       if (sireGroupBE != null) 
       { 
        //if the current group is in fact the seach group before saving 
        bool currentGroupIsSeachGroup = sireGroupBE.TheSireGroup.id == currentId; 

        //if we have setting this group as search group 
        bool selectedAsSearchGroup = this.chkBoxSelectedSireGroup.Checked; 

        //if the group we currently are in is not longer the seach group(chk box was unchecked) 
        bool wasSearchGroup = currentGroupIsSeachGroup && !selectedAsSearchGroup; 

        //if the group is becoming the search group 
        bool becomesSearchGroup = !currentGroupIsSeachGroup && selectedAsSearchGroup; 

        //if the group being deleted is in fact the search group 
        bool deletingSearchGroup = deletingGroup && currentGroupIsSeachGroup; 

        //if the user checked the checkbox but he's deleting it, not a so common case, but 
        //we shouldn't even consider to delete sire in this case 
        bool deletingTemporarySearchGroup = deletingGroup && !currentGroupIsSeachGroup;   

        //if we are not deleting a temporary search group and it's either 
        //becoming one (without deleting it) or we already are the search group 
        bool canDeleteSires = !deletingTemporarySearchGroup && 
              (becomesSearchGroup || currentGroupIsSeachGroup); 
        //we only delete sires if we are in search group 
        if (canDeleteSires) 
        { 
         if (deletingSearchGroup || wasSearchGroup) 
         { 
          // If we deleted all sires 
          deletedSire = new ArrayList(); 
          deletedSire.AddRange(sireGroupBE.SireList); 
         } 
         else 
         { 
          //if we delete a few sire from the change of search group 
          deletedSire = GetDeleteSire(sireGroupBE.SireList); 
         } 
        } 

        EndWaitMessage(dlg); 
        isClose = true; 
        result = ShowSubGroupAffected(deletedSire); 
       } 
      } 
      finally 
      { 
       if (!isClose) 
       { 
        EndWaitMessage(dlg); 
       } 
      } 
     } 

     return result; 
    } 

Answer 1

一つの選択肢は、ロジックの読み込み可能なバージョンを記述した名前のメソッドに主要ブール（canDeleteSires、deletingSearchGroup || wasSearchGroup）のそれぞれをリファクタリングすることです：

if (WeAreInSearchGroup()) 
{ 
    if (WeAreDeletingAllSires()) 
    { 
     deletedSire = new ArrayList(); 
     deletedSire.AddRange(sireGroupBE.SireList); 
    } 
    else 
    { 
     deletedSire = GetDeleteSire(sireGroupBE.SireList); 
    } 
} 

あなたはその後、これらのメソッド内で現在のブールロジックをカプセル化どのように状態（メソッドの引数やクラスメンバ）を渡すかは趣味の問題です。

これは、メインメソッドのブール値を直接質問したり答えたりする小さなメソッドに取り除きます。私は、このアプローチが "コメントは悪い"スタイルの開発で使われているのを見てきました。正直言って、もしあなたが孤独なオオカミであれば、これはちょっと残念ですが、チームで読むのがはるかに簡単です。

個人の好みのうち、私はまた、反転でしょう、あなたの最初の文は早く返すようにすれば、これは全体の方法のインデントレベル低下します：

if (searchAllSireList) 
{ 
    return result; 
} 

DataAccessDialog dlg = BeginWaitMessage(); 
bool isClose = false; 
try 
... 

をしかし、その後、あなたは、「複数のリターンで非難を受けるかもしれないです悪 "群衆。私は、これは、いくつかのインデントを取り除くための小さなリファクタリングです...

Answer 2

をリファクタリングする

方法。効率的ではありませんが、あなたが全面的に持っているブール値は、この機能を読みやすく分かりやすくします。

あなたは書いたものほど保守的ではありませんが、以下のようにすべてのboolを1行にまとめるような操作を行うことができます。

x =（（&b）&！d）| e;

Answer 3

おそらくすべてのコメントを削除してみることができます。あなたが持っているbool変数は、コードの理解に価値を追加しています。canDeleteSiresのインラインでそれらをいくつか入れても構いませんが、それで何らかの価値が追加されるとは思いません。

一方、そのコードはあなたのフォームにありますので、フォームをシンプルに保つことができ、コントローラーが実際にその動作を制御できるように、コントローラーではより良いかもしれません。

Answer 4

を開発練習は政治のようである印象を受ける：

private DialogResult CheckForSireRestrictionInSubGroup(bool deletingGroup,string currentId) 
{ 
    if (searchAllSireList) 
     return DialogResult.No; 

    DataAccessDialog dlg = BeginWaitMessage(); 
    bool isClose = false; 

    try 
    { 
     ISireGroupBE sireGroupBE = sireController.FindSireGroupSearch(); 

     if (sireGroupBE == null) 
      return DialogResult.No; 

     //if the current group is in fact the seach group before saving 
     bool currentGroupIsSeachGroup = sireGroupBE.TheSireGroup.id == currentId; 

     //if we have setting this group as search group 
     bool selectedAsSearchGroup = this.chkBoxSelectedSireGroup.Checked; 

     //if the group we currently are in is not longer the seach group(chk box was unchecked) 
     bool wasSearchGroup = currentGroupIsSeachGroup && !selectedAsSearchGroup; 

     //if the group is becoming the search group 
     bool becomesSearchGroup = !currentGroupIsSeachGroup && selectedAsSearchGroup; 

     //if the group being deleted is in fact the search group 
     bool deletingSearchGroup = deletingGroup && currentGroupIsSeachGroup; 

     //if the user checked the checkbox but he's deleting it, not a so common case, but 
     //we shouldn't even consider to delete sire in this case 
     bool deletingTemporarySearchGroup = deletingGroup && !currentGroupIsSeachGroup;   

     //if we are not deleting a temporary search group and it's either 
     //becoming one (without deleting it) or we already are the search group 
     bool canDeleteSires = !deletingTemporarySearchGroup && 
           (becomesSearchGroup || currentGroupIsSeachGroup); 

     ArrayList deletedSire = new ArrayList(); 

     //we only delete sires if we are in search group 
     if (canDeleteSires) 
     { 
      if (deletingSearchGroup || wasSearchGroup) 
      { 
       // If we deleted all sires 
       deletedSire.AddRange(sireGroupBE.SireList); 
      } 
      else 
      { 
       //if we delete a few sire from the change of search group 
       deletedSire = GetDeleteSire(sireGroupBE.SireList); 
      } 
     } 

     EndWaitMessage(dlg); 
     isClose = true; 
     return ShowSubGroupAffected(deletedSire); 
    } 
    finally 
    { 
     if (!isClose) 
     { 
      EndWaitMessage(dlg); 
     } 
    } 
    return DialogResult.No; 
}