這可以寫得更好嗎？根據以前的結果調用函數

我有以下用C＃編寫的代碼來檢查各種測試，如果有任何失敗，結果是錯誤的並且不希望其他檢查被執行。這可以寫得更好嗎？根據以前的結果調用函數

我編碼的方式看起來很「醜陋」，我想知道是否有一個更優雅的解決方案，我今天早上由於大腦衰退而失蹤。

//check the rules 
bool isValid = CheckPhoneFormat(); 
if (isValid) 
{ 
    isValid = CheckDoNotCall(); 
} 

if (isValid) 
{ 
    isValid = CheckStatusActive(); 
} 

if (isValid) 
{ 
    isValid = CheckOCV(); 
} 

if (isValid) 
{ 
    isValid = CheckCard(); 
} 

customer.IsValid = isValid;

來源

2014-10-27 jazza1000

屬於http://codereview.stackexchange.com – Shiva 2014-10-27 22:11:19

海事組織，你的做法是因爲它是可讀的，高效的，易於調試，測試和維護，因此絕對很好。不要用線或字母來衡量代碼質量。 – 2014-10-27 22:31:27

是：

bool isValid = CheckPhoneFormat() && CheckDoNotCall() 
       && CheckStatusActive() && CheckOCV() && CheckCard(); 
customer.IsValid = isValid;

的&&運算符計算只有在第一個是true第二操作數。

編輯
由於所有調用的方法參數的，可以考慮將它們轉換爲只得到的屬性（重命名）。它可能有助於調試。

bool isValid = IsValidPhoneFormat && IsValidDoNotCall 
       && IsValidStatusActive && IsValidOCV && IsValidCard; 
customer.IsValid = isValid;

來源

2014-10-27 22:08:34 Dmitry

我更喜歡OP的方法。調試更好。 – 2014-10-27 22:15:21

@TimSchmelter我同意，在這種情況下調試可能是一個問題。請參閱我的編輯。 – Dmitry 2014-10-27 23:14:06

您可以使用&&這將如果檢查失敗短路：

customer.IsValid = CheckPhoneFormat() && CheckDoNotCall() && CheckStatusActive() ...;

注意，你甚至不需要一個if，您可以直接分配給布爾值。

來源

2014-10-27 22:08:48 BradleyDotNET

我不喜歡這個，因爲它比OP的方法更糟糕。如果你不得不滾動，並且如果你不存儲返回值也很難調試。 – 2014-10-27 22:13:38

@TimSchmelter通常我會同意調試的難度，但是因爲他可以在每個函數中設置斷點，所以我會說它很簡單。就可讀性而言，您可以將每個條件放在一個新行中。也就是說，我知道你來自哪裏。 – BradleyDotNET 2014-10-27 22:15:41

是的，這使得它更加困難。你正在保存一些信件，但你失去了可測試性。您必須知道您必須檢查哪些方法，而不是設置快速折點。通過使用多行，它更具可讀性，但它也變得越來越類似於OP的方法，也更容易維護（如果你想記錄'CheckDoNotCall'返回'true'而不修改方法]。順便說一下，OP的方法也是短路。 – 2014-10-27 23:02:55

這增加的開銷用於創建一個狀態機：如果你願意

static Enumerable<Func<bool>() GetRules() 
{ 

    yield return CheckPhoneFormat(); 
    yield return CheckDoNotCall(); 
    yield return CheckStatusActive(); 
    yield return CheckOCV(); 
    yield return CheckCard(); 
} 

bool oneValid = GetRules().Any(b => b); 
bool allValid = GetRules().All(b => b);

更高效：

static IEnumerable<Func<bool>> arr = new Func<bool>[] 
{ 
    () => CheckPhoneFormat(), 
    () => CheckDoNotCall(), 
    () => CheckStatusActive(), 
    () => CheckOCV(), 
    () => CheckCard(); 
}; 

bool oneValid = arr.Any(b => b); 
bool allValid = arr.All(b => b);

來源

2014-10-27 22:08:51 abatishchev

我不會想到使用'yield'那樣的。非常聰明！ – BradleyDotNET 2014-10-27 22:09:56

喜歡聰明，但我認爲我會選擇短期評估作爲更簡單的解決方案 – jazza1000 2014-10-27 22:14:17

@ jazza1000：是的，你通過創建一個狀態機來增加一些開銷。我更新了我的答案。 – abatishchev 2014-10-27 22:19:44

這可以寫得更好嗎？根據以前的結果調用函數

回答

相關問題