哪种编码风格更好？

Question

在代码审查中，一位资深开发人员评论了我代码中的一些嵌套问题。他建议我设置一个bool值，以便我永远不会有超过一层的嵌套。我认为我的代码更容易阅读，但希望听听其他开发人员的意见。哪种风格更好？他对嵌套的反感是否合理？

以下是一些简化的代码示例。

嵌套：

If(condition1)
{
    If(condition2)
    {
        if(condition3)
        {
            return true;
        }
        else
        {
            log("condition3 failed");
        }
    else
    {
        log("condition2 failed")
    }
}
else
{
    log("condition1 failed")
}

return false;

或者

基于布尔值:

bool bRC = false;

bRC = (condition1);
if(brc)
{
    bRC = (condition2);
}
else
{
    log("condition1 failed");
    return false;
}

if(bRC)
{
    bRC = (condition3);
}
else
{
    log("condition2 failed");
    return false;
}

if(bRC)
{
    return true;
}
else
{
    log("condition3 failed");
    return false;
}

- BuckeyeSoftwareGuy

---

不要责怪你的高级开发人员；-) - JRoppert

我想我会选择选项2，因为使用选项1会导致编译错误。 - AndrewB

当您向高级开发人员展示此线程时，请报告他们的反应 :) - Peter Recore

好问题，讨论很多！ - Charles Bretana

嵌套版本并不是很好看 - 但布尔版本更加可怕。 - Jonathan Leffler

Answer 1

我喜欢你的方法更好，但我可能会这样做：

if (condition1 && condition2 && condition3)
{
    return true;
}
else if (!condition1)
{
    log("condition1 failed");
}
else if (!condition2)
{
    log("condition2 failed");
}
else
{
    log("condition3 failed");
}
return false;

如果条件是复杂表达式，我可能会在评估if语句之前将这些表达式分配给适当命名的变量，以避免在每个if中重新计算值。

这是假设所有条件都为真，并且您希望首先进行检查的正常模式。如果正常模式是一个或多个条件为假，则我会重新排序并逐个检查每个否定，并在所有检查失败时简单地返回true。这也会消除需要使用临时变量代替复杂表达式的需要。

- tvanfosson

---

我也赞同这个。 - nullArray

我喜欢这个，但如果条件中有函数，你可能会多次调用函数（除非你利用短路求值）。 - mauris

@Mauris -- 是的，但这可能是一种情况，我会评估条件并分配给本地变量，以避免重新计算它。 - tvanfosson

这是不等效的，因为如果任何一个条件为假，它不会返回false，所以它无法编译。它将会出现编译错误"并非所有代码路径都有返回"（或者类似这样的措辞）。 - Charles Bretana

@Charles -- 这仅仅是一小段代码，而不是整个函数，因此它本身无法编译通过。 为了避免混淆，我已经添加了返回值。 - tvanfosson

Answer 2

如果您没有任何有关多个返回点的荒谬规定，我认为这很不错（还有其他人也这样认为，但他们因未知原因删除了他们的答案）：

if(!condition1)
{
    log("condition1 failed");
    return false;
}

if(!condition2)
{
    log("condition2 failed");
    return false;
}

if(!condition3)
{
    log("condition3 failed");
    return false;
}

return true;

也许这是对于超级嵌套相等的本能反感，但它肯定比将布尔条件存储在某些值中更为清晰。然而，在上下文中可能不太易读：考虑如果其中一个条件是isreadable()。使用 if(isreadable()) 更清晰，因为我们想知道某个东西是否可读。而 if(!isreadable()) 暗示着我们想知道它是否不可读，这并不是我们的意图。当然有争议的是，在某些情况下，一个比另一个更易读/直观，但我自己喜欢这种方式。如果有人卡在返回值上，你可以这样做：

if(!condition1)
    log("condition1 failed");

else if(!condition2)
    log("condition2 failed");

else if(!condition3)
    log("condition3 failed");

else
    return true;

return false;

但那样做有点不诚实，而且在我看来不够“清晰”。

- Chris Lutz

---

你上一个方案的一个小变体：int rc = false; 如果和之前一样，那么就是if；否则rc = true; 函数结束时返回rc。没有什么不可告人的事情；在一切都正常之前，函数都会失败。 - Jonathan Leffler

这比布尔方法好多了，我会称之为模仿编程。它只是对复杂性的表面证据做出反应，但实现了一个同样复杂但更难以理解的替代方案，取代了嵌套结构，这种方法更糟糕。 - DigitalRoss

@Jonathan - 我认为使用不必要的变量几乎是不诚实的，但这可能并不重要。 - Chris Lutz

Answer 3

我个人认为嵌套代码更容易阅读。

- Michael Ekstrand

---

在我看来，更容易可视化程序流程。 - Gab Royer

我认为这样做要容易得多，而且更易于维护。 - Matt Davis

嵌套的方法会更少出错，而且可以减少需要使用额外布尔值的情况！ - mauris

如果嵌套开始变得太多，您可以将代码分解为多个函数。适度嵌套是可以的。 - Ates Goral

你需要一个宽屏幕。如果你想保持侧边栏的可见性，并且喜欢使用长的函数和变量名称，这一点尤其重要。 - Dimitri C.

Answer 4

通常我更喜欢使用嵌套的条件语句；当然，如果我的嵌套条件开始向右缩进得太深，我就必须开始考虑是否有更好的方法来完成我想要做的事情（重构、重新设计等）

- Adam Maras

Answer 5

与嵌套版本类似，但对我来说更加清晰简洁：

if not condition1:
    log("condition 1 failed")
else if not condition2:
    log("condition 2 failed")
else if not condition3:
    log("condition 3 failed")
else
    return true;
return false;

请注意，每个条件只会被评估一次。

- GogaRieger

---

+1 这相当于原始逻辑，而且更加简洁。 - Charles Bretana

我刚刚打了这个！我想我不需要发布它。+1 - DigitalRoss

Answer 6

第二种方式过于冗长：他真的建议确切地这样做吗？由于 else 部分有 return 语句，您不需要大多数 if 语句。 使用嵌套示例时，您依赖于不遗漏任何可能的 else 子句。 对我来说，这两种方式都不令人满意。

- user181548

---

每次看到"bResult"反模式我都会感到不适。 - Ates Goral

Answer 7

布尔驱动的方式很令人困惑。如果需要，嵌套是可以接受的，但你可以通过将条件合并为一个语句或调用一个方法来减少嵌套深度，或在其中进行一些进一步的评估。

- CRice

Answer 8

我认为两种方式都是可行的，各有优缺点。在嵌套非常深（例如8或10层）的情况下，我会使用布尔驱动风格。对于你这种只有三个级别的情况，我会选择你的样式，但对于上面的确切示例，我会这样写：

void YourMethod(...) { if (condition1 && condition2 && condition3) return true;

if (!condition1) log("condition 1 failed");

if (!condition2) log("condition 2 failed");

if (!condition3) log("condition 3 failed");

return result; }

或者如果您喜欢单一退出点（像我一样），可以这样写：

void YourMethod(...) { bool result = false;

if (condition1 && condition2 && condition3) { result = true; } else { if (!condition1) log("condition 1 failed");

if (!condition2) log("condition 2 failed");

if (!condition3) log("condition 3 failed"); } return result; }

这样，您将在第一次运行时记录所有失败的条件。在您的示例中，即使有多个失败的条件，您也只会得到一个失败的条件日志。

- JRoppert

---

多次冗余地检查条件可能不可取或不可能实现；分别可能存在性能问题或副作用。 - Ates Goral

Answer 9

我可能会选择

   if (!condition1)      log("condition 1 failed");
   else if (!condition2) log("condition 2 failed");
   else if (!condition3) log("condition 3 failed");
   // -------------------------------------------
   return condition1 && condition2 && condition3;

我认为这是等效的，并且更加简洁...

此外，一旦客户决定对所有条件进行评估和记录（而不仅仅是第一个失败的条件），则修改此内容以实现该目的将变得更加容易：

   if (!condition1) log("condition 1 failed");
   if (!condition2) log("condition 2 failed");
   if (!condition3) log("condition 3 failed");
   // -------------------------------------------
   return condition1 && condition2 && condition3;

- Charles Bretana

Answer 10

如果编程语言支持异常处理，我会选择以下方式：

try {
    if (!condition1) {
        throw "condition1 failed";
    }

    if (!condition2) {
        throw "condition2 failed";
    }

    if (!condition3) {
        throw "condition3 failed";
    }

    return true;

} catch (e) {
    log(e);
    return false;
}

编辑自Charles Bretana：请参阅使用异常进行控制流程

- Ates Goral

---

-1：风格上很可爱，但对于许多语言来说都是一个坏主意。例如，在Java中抛出（实际创建）异常的成本很高。 - Stephen C

如果在Java中费用高昂，那么在Objective-C中就是过分的。 - Justin Searls

你为什么认为每一行代码都需要关注性能呢？这很可能只是在启动时运行一次的初始化逻辑。具有多个故障点的复杂初始化函数常见于这种情况。在时间关键的向量处理函数中通常不会有三个故障点 -- 除非你的向量本身就有问题！ :) - Ates Goral

从给出的答案中，我诚实地认为你的最好。 - treznik

我把它删除了，因为将抛出异常作为控制流的替代方案是一种被广泛接受的反面模式，应该避免使用。即使代码每年只运行一次... - Charles Bretana