一个逗号表达式导致的BUG

前两天一个同事写了一个需求提测，测试的时候发现了一个BUG，然后他改完后我没太看懂就咨询了下是什么情况。

 

需求背景：

电脑内存小于某个条件了就做一个事情。

 

示例代码：

MEMORYSTATUSEX status;

GlobalMemoryStatusEx(&status);

 

DWORD GetMemLimitGB(const std::string& key, DWORD default = 1)

{

......

}

 

if ((static_cast<float>(status.ullTotalPhys) / 1024 * 1024 *1024) < GetMemLimitGB("xxxx"), 7)

{

......

}

 

第一眼看上去可能觉得括号有点多，然后再大概看看可能明白他的意思了，就是电脑的内存小于7GB了就做一个事情。

代码review的时候真是很难看出来问题。

 

再仔细看看，会发现问题。我把代码简化一下：

float f = (static_cast<float>(status.ullTotalPhys) / 1024 * 1024 *1024);

if (f < GetMemLimitGB("xxxx"), 7)

{

......

}

再简化一下：

bool b = f < GetMemLimitGB("xxxx");

if (b, 7)

{

......

}

 

这次一下就看出问题了，if条件中是一个逗号表达式，逗号表达式的结果取最后面一个表达式的值，也就是7，7就是一个true，所以if条件中的表达式永远为true。

那前面写的那些判断内存的逻辑都是没有效果的，最终导致了BUG。

 

如果规避：

1.当然是要用心写代码，特别是括号匹配一定是要成对的出现，一般写左括号的时候顺便把右括号也打出来，然后在括号里头填写代码。Review代码的时候也需要小心，因为不是你写的代码，不特别注意很难看出来。

2.尽量不要使用函数的默认值。默认值会导致写代码的时候容易偷懒，甚至不太理解默认值是什么意思就不写了。谷歌推荐函数重载来实现。具体参考：https://google.github.io/styleguide/cppguide.html#Default_Arguments

3.复杂的条件判断可以分开写，这样逻辑看起来就很清晰了，像上面的简化代码中  float f = (static_cast<float>(status.ullTotalPhys) / 1024 * 1024 *1024); 如果使用临时变量记录下来，那么if条件中的语句就简洁多了，也更容易理解和发现问题。