重構一個有太多bool的方法

Question

我在c＃中有這個方法，我希望重構它。有太多的布爾和線條。什麼是最好的重構。製作一個新班級看起來有點矯枉過正，簡單地減少兩個班似乎很難。任何洞察力或指針將不勝感激。

方法重構

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語句提前回國，這會降低整個方法的縮進級別：

if (searchAllSireList) 
{ 
    return result; 
} 

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

不過，你可能會被「多重收益得到懲戒的邪惡「的人羣。我得到的印象是發展的做法是像政治...

Answer 2

其實，我個人會離開它，因爲它是。儘管效率很低，但是你所有的布爾值都可以使這個函數易讀易懂。

你可以做一些事情，比如將所有的布爾結合到一行（如下），但是這不像你寫的那樣可維護。

x =（（a & b）&！d）| Ë;

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; 
}