不执行任何操作的 If 分支是代码味道还是良好实践?

发布于 2024-09-27 15:51:49 字数 745 浏览 1 评论 0原文

我已经回复了这里的帖子(或者至少评论了 )的答案包含这样的代码,但我想知道编写一系列 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 值表示不执行任何操作。

对我来说,这减少了代码的复杂性,并且当我看到它时就很有意义。那么,这种形式是好是坏(甚至是代码味道)?

I've responded to threads here (or at least commented) with answers containing code like this, but I'm wondering if it's good or bad form to write a series of if branches with one (or more) of the branches doing nothing in them, generally to eliminate checking for null in every branch.

An example (C# code):

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

instead of:

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

And, of course, this is just an example, as I tend to use these with larger and more complex classes. And in most of these cases, a null value would indicate to do nothing.

For me, this reduces the complication of my code and makes sense when I see it. So, is this good or bad form (a code smell, even)?

如果你对这篇内容有疑问,欢迎到本站社区发帖提问 参与讨论,获取更多帮助,或者扫码二维码加入 Web 技术交流群。

扫码二维码加入Web技术交流群

发布评论

需要 登录 才能够评论, 你可以免费 注册 一个本站的账号。

评论(8

孤云独去闲 2024-10-04 15:51:50

它是可读的,无论它是好是坏取决于您想要实现的目标 - 通常长嵌套的“goes-on-forever”类型 if 语句是不好的。不要忘记框架中内置的静态字符串方法:string.IsNullOrEmpty()string.IsNullOrWhiteSpace()

您的 if (str == null) { /* Do Nothing */ } 行很不寻常,但确实有一个积极的点:它让其他开发人员预先知道您故意为此不做任何事情如果您将其更改为长 if/else if 结构,那么您的意图可能会变得不清楚

if (str != null) 
{ 
    /* carry on with the rest of the tests */
}

It's readable, whether it is good or bad depends upon what you are trying to achieve - generally long nested "goes-on-forever" type if statements are bad. Don't forget about static string methods baked into the framework: string.IsNullOrEmpty() and string.IsNullOrWhiteSpace().

Your if (str == null) { /* Do nothing */ } line is unusual, but does have one positive point: it is letting other developers know up front that you are deliberately doing nothing for that case, with your long if/else if structure your intentions could become unclear if you changed it to

if (str != null) 
{ 
    /* carry on with the rest of the tests */
}
爱冒险 2024-10-04 15:51:49

我更喜欢这样做 -

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

我认为你总是可以将带有空主体的 if “改写”为带有主体的否定,并且它看起来更好并且更有意义。

I prefer doing it like this-

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

I think you can always "reword" an if with an empty body into it's negation with a body, and that it looks better and makes more sense.

很酷又爱笑 2024-10-04 15:51:49

我通常会在第一个 if 中放置一个 return 或类似的内容:

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

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

I would normally put a return or something like that in the first if:

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

If you can't do this, because the function does something else after the if/else, I'd say it's time to refactor, and split the if/else part out into a separate function, from which you can return early.

鲜血染红嫁衣 2024-10-04 15:51:49

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

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)
{
    // ...
}

我说这很奇怪,因为它混淆了你的意图并且违背了读者的期望。如果你检查某个条件,人们希望你在满足条件后做某事,但你却没有。这是因为您的意图并不是实际处理空条件,而是在检查您实际的两个条件时避免空指针em> 感兴趣。实际上,它不是要处理两个概念状态,而是具有健全性规定(非空输入),而是读起来就像您有三个 概念状态需要处理。事实上,从计算角度,你可以说存在三种这样的状态,但这不是重点——它不太清楚。

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

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

这只不过是一个增强可读性的风格问题,而不是一个问题代码气味。

编辑:但是,如果您设置了不执行任何操作的条件,我确实非常喜欢您添加“不执行任何操作”的评论。否则,人们可能会认为您只是忘记完成代码。

It is indeed good to avoid the following, because it needlessly re-checks one of the conditions (the fact that the compiler will optimize this away is beside the point--it potentially makes more work for folks trying to read your code):

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

But it's also rather bizarre to do what you suggested, below:

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

I say this is bizarre because it obfuscates your intent and defies the reader's expectations. If you check for a condition, people expect you to do something if it is satisfied--but you're not. This is because your intent is not to actually process the null condition, but rather to avoid a null pointer when you check the two conditions you're actually interested in. In effect, rather than having two conceptual states to handle, with a sanity provision (non-null input), it reads instead like you have three conceptual states to handle. The fact that, computationally, you could say there are three such states is beside the point--it's less clear.

The usual case approach in this sort of situation is as Oren A suggested--check for the null, and then check the other conditions within the result block:

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

This is little more than a matter of readability-enhancing style, as opposed to an issue of code smell.

EDIT: However, if you're set on the do-nothing condition, I do very much like that you included a "do nothing" comment. Otherwise, folks might think you simply forgot to complete the code.

魂牵梦绕锁你心扉 2024-10-04 15:51:49

在这种特殊情况下,我会提前返回,这使得代码更容易阅读

if (string.IsNullOrEmpty(str)) { return; }  

,我喜欢放置一个显式的 return 语句。

In this particular case I will return early and it makes code easier to read

if (string.IsNullOrEmpty(str)) { return; }  

I like to put an explicit return statement.

長街聽風 2024-10-04 15:51:49

是的这是一种代码味道。

一个迹象是您想问这个问题

另一个迹象是代码看起来不完整——好像有些东西应该属于那里。当然,它可能可读,但感觉不太好。

当阅读该代码时,外人必须停下来思考一下,并使用脑力来确定代码是否有效/完整/正确/符合预期/形容词。

user359996一语中的:

我说这很奇怪,因为它混淆了你的意图并违背了读者的期望。

Yes it is a code smell.

One indication is that you thought to ask this question.

Another indication is that the code looks incomplete- as if something should belong there. It may be readable sure, but it feels off.

When reading that code, an outsider has to stop for a second and use brainpower to determine if the code is valid/complete/correct/as intended/adjective.

user359996 hit the nail on the head:

I say this is bizarre because it obfuscates your intent and defies the reader's expectations.

小伙你站住 2024-10-04 15:51:49

你的第一个例子对我来说完全可读——根本没有味道。

Your first example is perfectly readable to me -- doesn't smell at all.

满天都是小星星 2024-10-04 15:51:49

这一切都取决于上下文。如果放置一个空的 if 语句可以使代码更具可读性,那么就这样做。

It all depends on context. If putting an empty if statement makes the code more readable, then go for that.

~没有更多了~
我们使用 Cookies 和其他技术来定制您的体验包括您的登录状态等。通过阅读我们的 隐私政策 了解更多相关信息。 单击 接受 或继续使用网站,即表示您同意使用 Cookies 和您的相关数据。
原文