翻译《有关编程、重构及其他的终极问题？》——39.为何不正确的代码可以工作

标签（空格分隔）：翻译 技术 C/C++
作者：Andrey Karpov
翻译者：顾笑群 - Rafael Gu
最后更新：2018年09月22日

39.为何不正确的代码可以工作

这次是在Miranda NG’s项目中发现的bug。PVS-Studio分析器对这个bug的诊断描述为：V502 Perhaps the ‘?:’ operator works in a different way than was expected. The ‘?:’ operator has a lower priority than the ‘|’ operator（译者注：大意是说这里的"?:"操作符可能不像预期的方式工作，“?:”操作符的优先级比“|”操作符低）。

#define MF_BYCOMMAND 0x00000000L
void CMenuBar::updateState(const HMENU hMenu) const
{
  ....
  ::CheckMenuItem(hMenu, ID_VIEW_SHOWAVATAR,
    MF_BYCOMMAND | dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED);
  ....
}

解释

我们已经看过了很多例子使得程序不能正常工作，这次我将引出一个不同思路的主题来讨论。有时候，我们发现了完全不正确的代码，但很奇怪的是，程序却工作正常！当然，这对于有经验的程序员来说，真不是一个奇怪的事情，但对于那些才刚学习C/C++的新手来说，就会有一些迷惑了。所以今天，我们来看看类似这些例子。

在上面所列出的代码总，我们需要调用CheckMenuItem()时传递了一些标志设置变量。第一眼看过代码，我们能发现如果bShowAvatar为true的话，我们就需要让MY_BYCOMMAND与MF_CHECKED位或操作；如果为false的话，就与MF_UNCHECKED位或操作，太简单了！

在上面的代码中，程序员很自然的使用了三元操作符来表达（这个操作符时if-then-else的缩写）：

MF_BYCOMMAND | dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED

但是，这里的“|”操作符的优先级高于“?:”操作符（详见C/C++的操作符优先级）。这样，我们立刻就能发现两个错误。

第一个错误就是判断条件已经被改变了，判断条件不再是一些人可能理解的“dat->bShowAvatar”了，而是“MF_BYCOMMAND | dat->bShowAvatar”。

第二个错误就是，只有MF_CHECKED或MF_UNCHECKED其中的一个标志被选择了，而标志MF_BYCOMMAND丢失了。

但即使有这些错误，代码还是工作正常的！原因就是运气太好了。这位程序员的运气就是MF_BYCOMMAND标志位0x00000000L。因为MF_BYCOMMAND为0，所以不会对代码有任何影响。可能很多有经验的程序员已经看出来了，但我依旧要说明一下，以防一些新手看到这里不明白。

首先，让我们看一下附加括号后正确的表达式：

MF_BYCOMMAND | (dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED)

使用实际的数字替换宏就变成：

0x00000000L | (dat->bShowAvatar ? 0x00000008L : 0x00000000L)

如果操作符“|”其中一个算子是0（译者注：即MF_BYCOMMAND，也即0x00000000L），我们就可以简化表达式为：

dat->bShowAvatar ? 0x00000008L : 0x00000000L

现在让我们更近一步的来看看错误的代码：

MF_BYCOMMAND | dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED

使用实际的数字替换宏就变成：

0x00000000L | dat->bShowAvatar ? 0x00000008L : 0x00000000L

在子表达式“0x00000000L | dat->bShowAvatar”，其中一个操作符“|”算子为0（译者注：即MF_BYCOMMAND，也即0x00000000L），我们同样可以简化表达式为：

dat->bShowAvatar ? 0x00000008L : 0x00000000L

结果我们得到了相同的表达式，这就是为何错误的代码可以正常工作——一个编程的奇迹发生了：）

正确的代码

有很多方法可以修正这段代码，其中一个就是增加括号，另外一种就是增加中间变量，用传统了if操作符也不错：

if (dat->bShowAvatar)
  ::CheckMenuItem(hMenu, ID_VIEW_SHOWAVATAR,
                  MF_BYCOMMAND | MF_CHECKED);
else
  ::CheckMenuItem(hMenu, ID_VIEW_SHOWAVATAR,
                  MF_BYCOMMAND | MF_UNCHECKED);

我不是一定要坚持使用这种方式修正代码。虽然这种方式更易读，但有一些太长。反正（使用什么修正方式）只和大家的喜好有关。

建议

我的建议很简单，就是避免使用复杂的表达式，特别是三元操作符。另外也不要忘了在合适的地方加括号。

其实我在前面的第4篇就提过“?:”是非常危险的，所以这里，我只是想提醒你这个操作符的优先级是很低的，所以也容易写出不正确的代码。大家往往也只是在想把代码写到一行的时候用它，所以不用就好了。