如此错误检查，还不如不要

今天在查一个Bug。表面现象很奇怪，至少让测试组觉得很奇怪。故障现象是这样的：
有一个列表框，如果里面只有两条记录，那么没事。如果有三条以上的记录，那么在删除记录时会“随机”出现程序无响应的情况。

上面这段错误描述中，“随机”二字之所以打引号，是因为其实不是随机的。只是测试组的同事没有找到规律而已。只要按从下到上的顺序进行逐个删除，就很容易遇到故障。

这种现象，有经验的程序员一看就知道跟序号、数组之类的东西有关。
果不其然，在代码中我找到了这样的一段：

pItem = ListView->Selected;
for (; pItem != NULL;)
{
    int iWhich = (int*)pItem->data;
    if (iWhich >=0 && iWhich < ItemArray.GetSize())
    {
        ItemArray.RemoveAt(iWhich);
        ListView->Delete(pItem->Index);
    }
    pItem = ListView->Selected;
}

这里解释一下，ListView->Selected能拿到列表内当前选中的第一行，而ItemArray是一个项目组自己实现的数组对象。这两个东西都没啥毛病。

坑爹的代码就是这一句：

if (iWhich >=0 && iWhich < ItemArray.GetSize())

我能理解，写的人是想把iWhich的取值限定在某个区间。因为这个iWhich接下来会被用作数组下标。如果越界，后果不堪设想。有这个意识，是好的。
但是，仅仅意识到这个问题，还不够。接下来还有问题了：iWhich会越界吗？什么情况下会？

我相信，如果写这段代码的人当时有问过自己这个问题，那么就不会有这个Bug了。
这个iWhich来自一个pItem->Data，这是一个ListView行对象附带的DWORD类型的数据，其值是由使用者（程序员）赋值的。
也就是说：如果你给它正确赋值，那么就不会有不正确的值出现。如果它有问题，那么是你前面的程序造成的。

也许还是有程序员会担心，这个pItem->Data会不会什么时候被改掉。也可能赋值的地方是另一个程序员写的。而写这段代码的程序员出于防御性编码的目的，写下了这样的判断。那么OK，没有关系，判断就判断吧。可是判断为FALSE的时候怎么办呢？
他什么都没有做。

其实，判断为FALSE之后，从ListView中删掉这一行，应该是安全的。从上面的代码中可以看出，删除行的时候，只用到了pItem->Index。这是由操作系统自行维护的值，不会存在Data那种“可能没有正确赋值”的情况。
又或者，实在不放心，那么直接把整个函数return掉，甚至报个错，也是可以的。
现在的处理方式就直接导致了死循环。这是很糟糕的情况，单核机器的用户甚至可能会几乎没有机会来做什么处理。

好了，上面这些就是我这篇博文想说的：有防御性编码的意识，很好！但是处理方式也要正确有效。你不应该在避免一个错误的同时，引入另一个错误。

那么，最后还有个遗留问题：iWhich为什么会越界呢？
我想，对于合格的程序员，这个问题不应该成为问题。所以我就不说了。