代码评审三步走
序言
在工作中参加过不同产品的多次代码检视活动，但在过程中观察到的一些现象引起了我的注意：不管是被检视者还是检视者，似乎都没有什么思路，只是对着代码发呆。即便能够发现一些问题，多数也是影响甚微的小问题。少数专家虽然能够发现有价值的问题，但也是凭经验+直觉。要问他们怎么做才能发现这些问题，却又没有答案。
为什么会有这样的现象呢？这种情况有没有办法加以改善呢？
我个人感觉，这是缺乏“套路”所致。研发活动，无论是架构设计，还是代码编写，或者测试活动，都有各自的方法或“套路”。这些“套路”能够指导相关人员有序地开展活动，并保证活动的有效性。那么代码检视是不是也应该有这样的“套路”可循呢？
经过一段时间的反复思考，我将自己的一些想法整理成此文，希望能够对代码检视活动提供一些帮助。
正文
下面将结合示例代码进行代码检视三步走的实战演示。
bool EnemyChecker::isValidMonster( NPCManagement::HordeIdType hrdId， const NPCManagement::MonsterInfoSeq& mnsInfos， NPCManagement::MonsterInfoSeq& vmnsInfos， NPCManagement::FailedMonsterInfoSeq& ivmnsInfos) { bool bRet = true; ivmnsInfos.length(mnsInfos.length()); vmnsInfos.length(mnsInfos.length()); unsigned int iip = 0; // index of invalid horde unsigned int ivp = 0; // index of valid horde //检查怪物是否属于部落群 for (CORBA::ULong i = 0; i < mnsInfos.length(); ++i) { const NPCManagement::MonsterStatusInfoSeq& mnsStatusSeq = mnsInfos[i].monsterInfos; for (CORBA::ULong i = 0; i < mnsInfos.length(); ++i) { const NPCManagement::MonsterStatusInfoSeq& mnsStatusSeq = mnsInfos[i].monsterInfos; IntSet monsterIds; for (CORBA::ULong j = 0; j < mnsStatusSeq.length(); ++j) { monsterIds.insert((int)mnsStatusSeq[j].monsterId); } if (monsterIds.empty()) { vmnsInfos[ivp].horde = mnsInfos[i].horde; vmnsInfos[ivp].monsterInfos.length(0); ++ivp; continue; }
std::map<int， short> mnsTypes; NPCManagement::MonsterLoader::getMonsterType(hrdId， -1， monsterIds， mnsTypes);
MonsterVec invalidMnsIds; getInvalidMnsIds(mnsStatusSeq， mnsTypes， invalidMnsIds); //存在部落群和怪物不一致或者怪物类型错误的情况 if ((monsterIds.size() != mnsTypes.size()) || !invalidMnsIds.empty()) { if (mnsTypes.empty())//一个有效怪物都没有时，记录无效怪物 { ivmnsInfos[iip].horde = mnsInfos[i].horde; ivmnsInfos[iip].monsters.length(mnsStatusSeq.length()); for (CORBA::ULong pos = 0; pos < mnsStatusSeq.length(); pos++) { ivmnsInfos[iip].monsters[pos].monsterId = mnsStatusSeq[pos].monsterId; ivmnsInfos[iip].monsters[pos].monsterName = mnsStatusSeq[pos].monsterName; ivmnsInfos[iip].monsters[pos].reason = NPC_STR2WSTR(WORLD::Message(getCvt()， "import_invalid_monster")); } iip++; } else { MonsterVec validMonsters; MonsterVec invalidMonsters; validMonsters.reserve(monsterIds.size()); for (IntSet::iterator it = monsterIds.begin(); it != monsterIds.end();++it) { MonsterVec::iterator invalidIt = std::find(invalidMnsIds.begin()， invalidMnsIds.end()， *it); //数据库中存在，且类型也有效 if ( invalidIt == invalidMnsIds.end() && mnsTypes.find(*it) != mnsTypes.end()) { validMonsters.push_back(*it); } else // 怪物Id和对应的怪物类型不一致或怪物不属于部落群 { invalidMonsters.push_back(*it); } } vmnsInfos[ivp].horde = mnsInfos[i].horde; vmnsInfos[ivp].monsterInfos.length((unsigned long)validMonsters.size()); for (unsigned int k = 0; k < validMonsters.size(); k++) { for (CORBA::ULong pos = 0; pos < mnsStatusSeq.length(); pos++) { if ((int)mnsStatusSeq[pos].monsterId == validMonsters[k]) { vmnsInfos[ivp].monsterInfos[k] = mnsStatusSeq[pos]; break; } } } ivmnsInfos[iip].horde = mnsInfos[i].horde; ivmnsInfos[iip].monsters.length((unsigned long)invalidMonsters.size()); for (unsigned int k = 0; k < invalidMonsters.size(); k++) { for (CORBA::ULong pos = 0; pos < mnsStatusSeq.length(); pos++) { if ((int)mnsStatusSeq[pos].monsterId == invalidMonsters[k]) { ivmnsInfos[iip].monsters[k].monsterId = (unsignedint)invalidMonsters[k]; ivmnsInfos[iip].monsters[k].monsterName = mnsStatusSeq[pos].monsterName; ivmnsInfos[iip].monsters[k].reason = NPC_STR2WSTR(WORLD::Message(getCvt()， "import_invalid_monster")); break; } } } ivp++; iip++; } } else//部落群和怪物一致，且对应怪物类型一致 { vmnsInfos[ivp].horde = mnsInfos[i].horde; vmnsInfos[ivp].monsterInfos = mnsStatusSeq; ivp++; } } ivmnsInfos.length(iip); vmnsInfos.length(ivp); return bRet; }
示例代码段一：
第一步：
代码检视要做的首先是评审代码的逻辑正确性，即代码是否完整、准确地实现了业务逻辑。
这一步只需对代码进行抽象，将其“翻译”为“注释”可阅读其实现逻辑，并与需求逻辑（设计逻辑）相比较。
这第一步很重要，而且实践难度其实不高。对于有“注释驱动编码”习惯的开发人员来说更是容易。
对示例代码段一，“翻译”成简略的“注释”（有省略），可以如下文（以大括号{之后紧接的为第一行）：
