是一个If分支什么都不做代码味道或良好实践？

Question

我已经回复了这里的线程(或者至少评论过)包含这样的代码的答案,但我想知道编写一系列if分支是否好或坏的形式,其中一个(或多个)分支在其中不执行任何操作,通常是为了消除null每个分支的检查.

一个例子(C#代码):

if (str == null) { /* Do nothing */ }
else if (str == "SomeSpecialValue")
{
    // ...
}
else if (str.Length > 1)
{
    // ...
}

代替:

if (str != null && str == "SomeSpecialValue")
{
    // ...
}
else if (str != null && str.Length > 1)
{
    // ...
}

当然,这只是一个例子,因为我倾向于使用更大更复杂的类.在大多数情况下,一个null值表示什么都不做.

对我来说,这可以减少我的代码的复杂性,并在我看到它时有意义.那么,这是好还是坏的形式(代码味道,甚至)？

Answer 1

我更喜欢这样做 -

if (str != null) 
{
 if (str == "[NULL]")
 {
    // ...
 }
 else if (str.Length > 1)
 {
    // ...
 }
}  

我认为你总是可以if用一个空的身体"改写"一个身体的否定,并且它看起来更好,更有意义.

Answer 2

我通常会在第一个中添加return或类似的东西if:

void Foo()
{
  if (str == null) { return; }
  if (str == "SomeSpecialValue")
  {
      // ...
  }
  else if (str.Length > 1)
  {
      // ...
  }
}

如果你不能这样做,因为函数在if/ 之后执行了其他操作else,我会说是时候重构,并将if/ elsepart 拆分成一个单独的函数,从中可以提前返回.

Answer 3

避免以下情况确实是好的,因为它不必要地重新检查其中一个条件(编译器将优化它的事实不是重点 - 它可能会为尝试读取代码的人们提供更多工作):

if (str != null && str == "SomeSpecialValue")
{
    // ...
}
else if (str != null && str.Length > 1)
{
    // ...
}

但是,按照你的建议行事也很奇怪,如下:

if (str == null) { /* Do nothing */ }
else if (str == "SomeSpecialValue")
{
    // ...
}
else if (str.Length > 1)
{
    // ...
}

我说这很奇怪,因为它混淆了你的意图并且无视读者的期望.如果你检查一个条件,人们希望你做一些事情,如果它满意 - 但你不是.这是因为你的意图不是实际处理 null条件,而是在检查你真正感兴趣的两个条件时避免使用空指针.实际上,不是要有两个概念状态来处理,而是提供一个完整的条件(非空输入),它读取就像你有三个概念状态要处理.事实上,在计算上,你可以说有三种这样的状态是不重要的 - 它不太清楚.

在这种情况下通常的案例方法是Oren A建议的 - 检查null,然后检查结果块中的其他条件:

if (str != null) 
{
 if (str == "SomeSpecialValue")
 {
    // ...
 }
 else if (str.Length > 1)
 {
    // ...
 }
}

这仅仅是提高可读性的风格,而不是代码嗅觉的问题.

编辑:但是,如果你对什么也不做的条件设置,我做的很像,你收录了"什么都不做"的评论.否则,人们可能会认为你只是忘了完成代码.