2010-03-02 58 views
3

我有一個驗證方法,它有很多條件語句。基本上它是有很多條件返回語句的重構方法

If Check1 = false 
    return false 
If Check2 = false 
    return false 
etc 

FxCop抱怨圈複雜度太高。我知道在函數中間返回聲明並不是最佳實踐,但同時我看到的唯一選擇是If-else語句的醜陋列表。解決這個問題的最好方法是什麼?

在此先感謝。

回答

11

我不同意你的說法,即在方法中間使用return語句不是最佳做法。爲了獲得單個返回語句,一些人需要花費的時間非常長 - 使用任何能夠產生最具可讀性的代碼。有時候這只是一個單一的回報點,但是我經常發現有一個「提前退出」 - 最好有這樣的回報,而不是爲替代路徑引入更多的嵌套代碼(if)。作爲一種經驗法則,我喜歡不會過分縮進太多的方法:)

話雖如此,方法確實是沒有什麼但檢查?檢查是否獨立?他們需要什麼變量?你可以將它們分組爲代表你正在執行的檢查的「區域」的較小方法嗎?使用

+0

你可以把它想象成驗證用戶輸入到很多領域。一些支票是獨立的,其他的則不是。你是對的,它可以引入許多嵌套的if語句和縮進代碼,如果只使用單個return語句。 – 2010-03-02 20:21:18

+1

我認爲最好的做法只有一個返回語句來自純C代碼,其中手動釋放資源。在這種情況下,單個返回點簡化了資源處理。在使用垃圾收集的現代語言中,它不再相關,所以我完全同意上述內容。 – 2010-03-02 20:24:49

+0

@Anders:垃圾收集和嘗試/最後或使用語句當然允許更清晰的清理 - 我想你是正確的關於曼陀羅的來源。只是另一個知道*爲什麼*某些事情被認爲是好事的案例,並對其他情況進行重新評估。很重要。 – 2010-03-02 20:38:41

1

你的例子:

return (Check1 == false) || (Check2 == false) [ || etc ] 
1

爲您的特定情況下,似乎你很可能使用循環......這個假設出來的事件處理程序的窗體或東西:

For Each ctrl As Control In Me.Controls 

    Dim check As CheckBox = TryCast(ctrl, CheckBox) 

    If check IsNot Nothing AndAlso check.Checked = False Then 
     Return False 
    End If 

Next 

Return True 

編輯:在進一步審查後,我意識到這是基於psuedocode,你實際上並沒有使用複選框。 但是,我認爲同樣的方法適用。你可以很容易地將你的規則重構爲一個獨立的類,它實現了一個自定義的接口(IRule或類似的),或者有一個代理列表,它返回true/false,你將作爲你的檢查模式的一部分進行迭代。

+0

一旦你的條件陳述變得越來越複雜,這是正確的方法。這個答案中提供的複選框示例非常聰明 - 爲什麼你不應該有運行時控制你的代碼運行的驗證器集合。另一個常見的情況是,當你想返回驗證失敗的原因時,更好地隱藏實際條件邏輯中的那種東西。 – CurtainDog 2010-06-16 00:30:28

0

如果連續多次檢查,不需要保留短路,你可以嘗試這樣的事:

 bool isValid = true; 
     isValid &= Condition1; 
     isValid &= Condition2; 
     isValid &= Condition3; 
     if (!isValid) 
      return false; 

如果您確實需要保留短路,您可以整合條件變成大if語句。但是,如果有很多條件,我寧願使用多個返回的原始代碼,因爲一個大的'if'可能會有點難看。

需要注意的是,在後一種情況下,你只是側步執行指標,作爲邏輯分支實際上是一樣的。如果條件檢查沒有副作用(我真的希望他們不這樣做),那麼我認爲高複雜度是一個虛驚,因爲多重返回實際上只是表達複雜布爾驗證邏輯的一種更可讀的方式。

然而,只有條件都是連續的,這纔是真實的。如果它們遍佈整個代碼,並且在檢查之間發生有意義的處理,那麼度量標準指示真正的複雜性(更多分支必須被測試)。在這種情況下,您可能會考慮該方法是否可以邏輯分解爲更小,單獨可測試的塊。