1

我写了一些类似于以下的代码:

String SKIP_FIRST = "foo";
String SKIP_SECOND = "foo/bar";

int skipFooBarIndex(String[] list){
    int index;
    if (list.length >= (index = 1) && list[0].equals(SKIP_FIRST) ||
        list.length >= (index = 2) && 
        (list[0] + "/" + list[1]).equals(SKIP_SECOND)){
        return index;
    }

    return 0;
}

String[] myArray = "foo/bar/apples/peaches/cherries".split("/");
print(skipFooBarIndex(myArray);

这通过分配索引来更改 if 语句内部的状态。然而,我的同事非常不喜欢这个。

这是一种有害的做法吗?有什么理由这样做吗?

4

14 回答 14

15

是的。这明显降低了可读性。以下代码有什么问题?

int skipFooBarIndex(String[] list){
    if(list.length >= 1 && list[0].equals(SKIP_FIRST)) 
        return 1;
    if(list.length >= 2 && (list[0] + "/" + list[1]).equals(SKIP_SECOND))
        return 2;
    return 0;
}

这更容易理解。一般来说,不鼓励在表达式中产生副作用,因为您将依赖子表达式的求值顺序。

假设您将其视为“聪明”的代码,最好永远记住 Brian Kernighan 的名言:

首先,调试的难度是编写代码的两倍。因此,如果您尽可能巧妙地编写代码,那么根据定义,您还不够聪明,无法对其进行调试。

于 2009-11-25T22:33:31.963 回答
4

......但是,我的同事非常不喜欢这个......

是的。不仅仅是因为你可以这样编码,你必须这样做。

请记住,那段代码最终必须由某人维护(某人可能在 8 个月后成为你自己)

更改 if、make 中的状态更难阅读和理解(主要是因为它不常见)

引用马丁·福勒的话:

任何傻瓜都可以编写计算机可以理解的代码。优秀的程序员编写人类可以理解的代码

于 2009-11-25T22:49:11.567 回答
2

不这样做有一个很好的理由:它让你的代码很难理解和推理。

于 2009-11-25T22:33:51.687 回答
2

问题是代码会在代码审查会话中生成多个 WTF。任何让人们“等等,什么?”的东西。得走了。

可悲的是,即使在易于阅读的代码中也很容易创建错误。没有理由让它变得更容易。

于 2009-11-25T22:35:16.157 回答
1

是的,在审查代码时很难跟踪副作用。

关于这样做的理由:不,没有真正的理由这样做。我还没有偶然发现一个 if 语句不能在没有副作用的情况下重写而不会造成任何损失。

于 2009-11-25T22:33:38.067 回答
1

唯一的问题是它对那些不写它的人来说是陌生和困惑的,至少在他们弄明白的一分钟内。我可能会这样写以使其更具可读性:

if (list.length >= 1 && list[0].equals(SKIP_FIRST)) {
    return 1;
}

if (list.length >= 2 && (list[0] + "/" + list[1]).equals(SKIP_SECOND)) {
    return 2;
}
于 2009-11-25T22:36:16.413 回答
1

从cppreference.com借来的:

C++ 与运算符优先级相关的一个重要方面是表达式中的求值顺序和副作用的顺序。在某些情况下,事情发生的顺序是没有定义的。例如,考虑以下代码:

float x = 1;
x = x / ++x;

x 的值不能保证在不同的编译器之间保持一致,因为不清楚计算机应该先计算除法的左边还是右边。根据首先评估哪一侧,x 可以取不同的值。

此外,虽然 ++x 的计算结果为 x+1,但将新值实际存储在 x 中的副作用可能发生在不同的时间,从而导致 x 的值不同。

最重要的是,像上面这样的表达方式非常模棱两可,应该不惜一切代价避免。如有疑问,请将单个模棱两可的表达式分解为多个表达式,以确保评估顺序正确。

于 2009-11-25T22:45:02.523 回答
1

这是一种有害的做法吗?

绝对没错。代码很难理解。除了作者之外,任何人都需要阅读两三遍。任何难以理解的代码,并且可以以更容易理解的更简单的方式重写,都应该以这种方式重写。

你的同事是绝对正确的。

有什么理由这样做吗?

这样做的唯一可能原因是您已经广泛地分析了应用程序并发现这部分代码是一个重要的瓶颈。然后你实现了上面的可憎,重新运行分析器,发现它确实提高了性能。

于 2009-11-26T00:26:16.397 回答
0

好吧,我花了一些时间阅读上面的内容,却没有意识到发生了什么。所以我肯定会建议这并不理想。我真的不会期望 if() 语句本身会改变状态。

于 2009-11-25T22:37:41.487 回答
0

如果没有很好的理由,我不会推荐具有副作用的 if 条件。对我来说,这个特殊的例子需要看几次才能弄清楚发生了什么。可能有一种情况并不那么糟糕,尽管我当然想不出一个。

于 2009-11-25T22:38:02.557 回答
0

理想情况下,每段代码都应该做一件事。让它做不止一件事可能会造成混淆,而混淆正是您不希望在代码中出现的内容。

if 语句条件中的代码应该生成一个布尔值。分配一个值的任务是让它做两件事,这通常是不好的。

此外,人们期望条件只是条件,当他们对代码正在做什么有印象时,他们经常会浏览它们。在他们决定需要之前,他们不会仔细解析所有内容。

将其粘贴在我正在审查的代码中,我会将其标记为缺陷。

于 2009-11-25T22:44:31.400 回答
0

在 C 中,在 if 语句中更改状态是相当普遍的。一般来说,我发现有一些不成文的规定可以接受,例如:

  • 您正在读取一个变量并检查结果:

    int a;
    ...
    if ((a = getchar()) == 'q') { ... }
    
  • 增加一个值并检查结果:

    int *a = (int *)0xdeadbeef;
    ...
    if (5 == *(a++)) { ... }
    

当它不可接受时:

  • 您正在将常量分配给变量:

    int a;
    ...
    if (a = 5) { ... } // this is almost always unintentional
    
  • 混合和匹配前增量和后增量,以及短路

    int a = 0, b;
    ...
    if (b || a++) { ... }    // BAD!
    

出于某种原因,我试图标记为代码的部分的字体在 SO 上不是固定宽度的,但是在固定宽度的字体中,存在if表达式内部分配既合理又清晰的情况。

于 2009-11-25T23:16:29.927 回答
0

您还可以获得三元以避免多次返回:

int skipFooBarIndex(String[] list) {
    return (list.length > 0 && list[0].equals(SKIP_FIRST)) ? 1 :
       ((list.length > 1 && (list[0] + "/" + list[1]).equals(SKIP_SECOND)) ? 2 : 0);
}

虽然这个例子可读性较差。

于 2009-11-25T23:32:29.597 回答
0

作为一个做很多维护编程的人说话:如果我遇到这个,我会诅咒你,哭泣然后改变它。

像这样的代码是一场噩梦——它尖叫着两件事之一

  1. 我是新来的,我需要帮助做正确的事情。
  2. 我认为我很聪明,因为我节省了几行代码,或者我欺骗了编译器并让它更快。它不聪明,不是最佳的,也不好笑

;)

于 2009-11-25T23:55:48.647 回答