概述
在實際的軟件開發項目中，代碼評審是一個必不可少的流程。代碼評審，也稱之為代碼復查，是指通過閱讀開發人員所寫的代碼來檢查源代碼與編碼規范的符合性以及代碼質量的活動。總的說來，代碼評審的好處有以下幾點：
第一，發現程序問題，提高代碼質量。
第二，理清代碼邏輯，開闊編程思路。
第三，促進團隊交流，提升開發技能。
代碼評審的大體流程是這樣的：
第一步，團隊負責人(通常是開發經理)提前預定好會議室，並通知參與代碼評審的人員，讓他們做好准備。
第二步，在評審會上，講解員大聲閱讀被評審的代碼，評審人員就代碼的問題給出自己的看法和意見，記錄員再將這些意見記錄下來。
第三步，評審結束之後，團隊負責人將評審記錄以郵件的形式發出來，並督促被評審代碼的編寫者按照評審意見對代碼進行修改。
代碼評審小結
最近，我參加了幾個C語言代碼的評審會，發現了代碼中一些共同存在的問題，特總結下來，供相關的開發人員參考。
1.添加、修改、刪除代碼的時候缺少了注釋說明。
在很多開發人員看來，注釋是可有可無的東西，只要實現了程序的功能即可。實際上，這種想法是不對的。對於缺少必要注釋的代碼，在剛編寫的時候，還能夠知道每行代碼的作用，但是當經過了多個版本的演進之後，不要說其他人，就算是作者本人，也未必知道當初自己為何要這麼編寫代碼。當你接手缺少注釋而讀起來很吃力的代碼的時候，有一種想把代碼原作者拉過來打一頓的沖動。不管你又沒有，反正我是有的。
因此，不管工作有多忙，在編寫代碼的同時，我們一定要在適當的地方添加適當的注釋，這也是對一個合格的編程人員的基本要求。
2.在代碼關鍵分支處缺少了日志信息，不便於分析和排查問題。
很多代碼在“return”語句之前或else分支處沒有日志信息，這使得跟蹤代碼流程變得比較困難，在程序出現故障的時候不便於分析和排查問題。
例如，如下代碼：
if (var1 > 0)
{
return;
}
if (var2 > 0)
{
return;
}
if (var3 > 0)
{
return;
}
當程序執行到這段代碼而返回的時候，我們不知道到底是哪個變量大於了0，這導致我們不知從哪裡入手排查問題。
正確的做法是在每個“return”語句之前，都添加詳細的日志說明，如下語句所示：
if (var1 > 0)
{
// 日志說明1
return;
}
if (var2 > 0)
{
// 日志說明2
return;
}
if (var3 > 0)
{
// 日志說明3
return;
}
又如，如下代碼：
if (var1 > 0)
{
// 執行操作1
}
else
{
return;
}
if (var2 > 0)
{
// 執行操作2
}
else
{
return;
}
if (var3 > 0)
{
// 執行操作3
}
else
{
return;
}
當程序執行到這段代碼而返回的時候，我們不知道到底是在哪個else分支返回的。正確的做法同上，要在每個“return”語句之前，都添加詳細的日志說明。
3.代碼中出現了魔幻數字，不知道它們表達的意思。
例如，如下代碼：
if (var1 == 100)
{
// 執行操作1
}
else if (var1 == 200)
{
// 執行操作2
}
else if (var1 == 300)
{
// 執行操作3
}
else
{
// 執行操作4
}
在看到這段代碼的時候，我們不知道代碼中的100、200、300表示什麼意思，只是感覺頭有點暈。這類我們不知道表達什麼意思的數字就統稱為魔幻數字。
為了消滅魔幻數字，正確的做法是將它們用宏替代，以提高代碼的可閱讀性和可修改性。將上面的代碼修改之後如下所示：
#define SMALL 100
#define MEDIUM 200
#define BIG 300
if (var1 == SMALL)
{
// 執行操作1
}
else if (var1 == MEDIUM)
{
// 執行操作2
}
else if (var1 == BIG)
{
// 執行操作3
}
else
{
// 執行操作4
}
4.在使用字符串相關操作函數strcpy、memcpy等的時候，沒有判斷拷貝長度。
在涉及到字符串相關操作的時候，大家一定注意防止操作不當而引起內存越界。如下代碼沒有判斷拷貝長度，就容易引起了內存越界：
char src[100] = {0};
char dest[80] = {0};
strcpy(dest, src);
如上面的代碼所示，當src中字符串的長度大於了80的時候，就會出現內存越界的情況，程序就會崩潰。
正確的做法是在執行拷貝操作之前，先判斷字符串的長度，同時將strcpy函數替換為strncpy。修改之後的代碼如下所示：
char src[100] = {0};
char dest[80] = {0};
int len = 0;
if (strlen(src) >= sizeof(dest)-1)
{
len = sizeof(dest)-1;
}
else
{
len = strlen(src);
}
strncpy(dest, src, len);
5.if…else語句的判斷條件不明確，用多個變量作為多個else if的判斷條件。
例如，如下代碼所示：
#define SMALL 100
#define MEDIUM 200
#define BIG 300
if (var1 == SMALL)
{
// 執行操作1
}
else if (var1 == MEDIUM)
{
// 執行操作2
}
else if (var2 == BIG)
{
// 執行操作3
}
else if (var3 == BIG)
{
// 執行操作4
}
else
{
// 執行操作5
}
在上面的代碼中，if…else語句的判斷條件中使用了三個不同的變量var1、var2和var3，這導致了代碼閱讀起來不方便，而且後續對代碼的修改也容易出現錯誤。
正確的做法是同一級的if…else語句的判斷條件中要使用相同的變量，修改之後的代碼如下所示：
#define SMALL 100
#define MEDIUM 200
#define BIG 300
if (var1 == SMALL)
{
// 執行操作1
}
else if (var1 == MEDIUM)
{
// 執行操作2
}
else
{
if (var2 == BIG)
{
// 執行操作3
}
else
{
if (var3 == BIG)
{
// 執行操作4
}
else
{
// 執行操作5
}
}
}
6.定義變量的時候排版不工整。
在編寫幾乎每一個函數的時候，我們都會定義一些變量，包括整型變量、字符型變量、指針變量、結構體變量等。如果把這些變量放在一起而不注意排版，那麼看起來會非常的亂。如下代碼所示：
char str1[50] = {0};
int i = 0;
int max = 0;
char str2[500] = {0};
char *pStr = NULL;
TestStruct_T tTestStruct = {0};
int j = 0;
char str3[1000] = {0};
從上面的代碼可以看出，將不同的變量混合放在一起，代碼看起來很凌亂，嚴重影響了程序的可閱讀性。為此，我們要將同種類型的變量放在一起，利用縮進來讓它們對齊，並利用空行來分隔不同類型的變量。修改之後的代碼如下所示：
int i = 0;
int j = 0;
int max = 0;
char str1[50] = {0};
char str2[500] = {0};
char str3[1000] = {0};
char *pStr = NULL;
TestStruct_T tTestStruct = {0};
這樣修改之後的代碼閱讀起來，層次感更好，可閱讀性更強。
7.沒有判斷函數輸入參數(尤其是指針變量)的合法性。
在定義函數的過程中，我們很容易忘記對它的輸入參數的合法性進行校驗，這樣也就增大了程序出問題的風險。如下代碼所示：
void TestFun(char *pMsg)
{
char buf[1000] = {0};
memcpy(buf, pMsg, sizeof(buf)-1);
// 執行後續流程
}
可以看到，在函數TestFun中，我們直接將pMsg拷貝到buf中，而沒有判斷其是否為空。如果pMsg為空，那麼這個拷貝就會出現問題，程序就會崩潰。
正確的做法是在使用輸入指針pMsg之前，先對其合法性進行校驗，也就是判斷它是否為空。如果為空，那麼直接返回，不進行後續處理。修改之後的代碼如下：
void TestFun(char *pMsg)
{
char buf[1000] = {0};
if (pMsg == NULL)
{
printf(“TestFun: pMsgis NULL!\n”);
return;
}
memcpy(buf, pMsg, sizeof(buf)-1);
// 執行後續流程
}
總結
不管是開發什麼樣的軟件產品，只要需要寫代碼，那麼就離不開代碼評審。通過代碼評審，我們不僅可以很快發現代碼中存在的問題，而且可以發現自己思維的缺陷及在技術上的欠缺，這對提升開發人員本身的素質也是很有好處的。
在代碼評審的時候，大家要本著實事求是的態度，不要因為怕傷了和氣而不好意思將問題講出來，更不要由代碼問題而轉向對開發人員的人身攻擊(例如說某人寫的代碼太爛了)。我們始終要記住大家的共同目標：將產品做好。
