这个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启发 - 功能应该做的一件事，他们应该把它做好他们应该只是做（或类似的东西） - 如果有人想知道，。））