這個IF語句代碼片斷如何被重構？

Question

我已經閱讀了很多重構文獻，並且在例子中看到的一個常見主題是大量IF語句的減少。下面是一個非常簡單的代碼塊，首先檢查是否提供了電子郵件地址，如果是，則繼續驗證電子郵件地址。我通常會看到很多這種類型的代碼，看起來總是有點混亂，特別是如果有多個變量的話。

if (string.IsNullOrEmpty(email)) { 

    throw new ApplicationException("Email Address Required"); 
} 

else { 

    if (!ValidationService.EmailAddressIsValid(email)) { 
     throw new ApplicationException("Invalid Email Address"); 
    } 

} 

我的問題是，這個例子是完全可以接受的嗎？它有味道嗎？如何重構此片段？

Answer 1

很難重構這樣一個小片段 - 我會跳過關於不重構而不進行測試通常的說法...

當我看到這樣的代碼，我認爲

• 爲什麼ValidationService不保存空檢查的邏輯嗎？如果這不在包含驗證邏輯的類中，那麼爲什麼我們要在這裏檢查必填字段？

• 我很可能會令空支票存入一個幫手所以代碼可能會更喜歡：

  ThrowIfRequiredFieldMissing(email, "Email Address");

• 除去別的，因爲扔無法繼續

• 創建一個幫手投擲：FailIf(!Validation.EmailAdressIsValid(email), "Email Address")。這將引發你的驗證異常，並允許在需要時您可以在以後更改例外（雖然我會船到橋頭當你來到它）

還有許多其他的可能性，像Func<>本詞典，或應用於字段列表的戰略模式，以及等等。 但是沒有更多的代碼，決定做什麼（如果有的話）只是猜測。

Answer 2

拋永遠不會返回，所以我會用更好的可讀性如下：

if (string.IsNullOrEmpty(email)) { 
    throw new ApplicationException("Email Address Required"); 
} 

if (ValidationService.EmailAddressIsValid(email)) { 
    throw new ApplicationException("Invalid Email Address"); 
} 

// continue here if no error was detected 

Answer 3

這很好，但你可以更簡化它。

if (string.IsNullOrEmpty(email)) { 
    throw new ApplicationException("Email Address Required"); 
} else if (ValidationService.EmailAddressIsValid(email)) { 
throw new ApplicationException("Invalid Email Address"); 
} 

Answer 4

你可以做的第一件事是刪除行else {及其相應}。

這是因爲如果電子郵件地址丟失，第一個throw語句將確保當前位於else塊中的代碼永遠不會執行。

Answer 5

重構不僅僅是一小段代碼樣式。

您正在驗證用戶條目，因此您可能需要定義或重新使用現有的驗證框架，而不是在代碼中嵌入if語句。你好像差不多了，因爲你有ValidationService進行第二次驗證。

所以我的建議是：在執行EmailAddressIsValid時加入string.IsNullOrEmpty檢查。

當然這隻有移動一個if語句，但如果在多個地方使用EmailAddressIsValid它將減少if語句。此外，驗證邏輯然後真正整合在一個地方。目前不是。

另一件小事：重命名EmailAddressIsValid到IsEmailAddressValid，因爲使用「Is」作爲返回布爾值的函數的前綴是一個常見的約定。

Answer 6

醜陋的變異，但可行

string msg = string.Empty;  
if (string.IsNullOrEmpty(email)) 
{ 
    msg = "Email Address required"; 
} 
else if (ValidationService.EmailAddressIsValid(email)) 
{ 
    msg = "Invalid Email Address"; 
} 

if (msg != string.Empty()) 
    throw new ApplicationException(msg); 

Answer 7

，如果您真的想使你的代碼的可讀性，你甚至可以做這樣的事情：

throwExceptionIfEmailIsEmpty(email); 
throwExceptionIfEmailIsInvalid(email); 
// continue to process the e-mail here... 

隨着提取到單獨的驗證碼功能：

private void throwExceptionIfEmailIsEmpty(String email) 
{ 
    if (string.IsNullOrEmpty(email)) 
     throw new ApplicationException("Email Address Required"); 
} 

private void throwExceptionIfEmailIsInvalid(String email) 
{ 
    if (ValidationService.EmailAddressIsValid(email)) 
     throw new ApplicationException("Invalid Email Address"); 
} 

它可能看起來有點過分，但它使得查看真正發生的事情變得更容易！
（鮑勃叔叔的Clean Code啓發 - 功能應該做的一件事，他們應該把它做好他們應該只是做（或類似的東西） - 如果有人想知道，。））