Code Review是軟體開發過程中非常重要的一個環節，其主要的目的是在項目早期找到和修正錯誤、提升軟體品質。 複制
本文主要關注的是在做Code Review的時候，我們主要在關心代碼的哪些方面來進行說明。
每個人的關注點不盡相同，于我而言，我的關注點一般在下面的幾個部分上：
- 基礎篇 - 包括編碼規範、風格、日志規範、記憶體洩漏等
- 進階篇 - 包括是否有較好的抽象、資料庫變更檢查等
- 高階篇 - 包括應急方案、失敗性考慮等
接下來，我們逐一來看一下。
基礎篇
基本編碼規範
- 類、方法命名是否規範、清晰
- 方法參數是否過多（比如6個以上）、方法體是否過大
- 重複代碼
- 是否有“魔數”，比如 == 1 處理什麼邏輯？(建議有良好的注釋說明，定義常量或枚舉代替)
- ... ...
日志規範
- 日志列印是否有意義，比如像如下幾種，這種日志對問題排查等沒有啥意義
logger.info("方法xxx執行");... ...
logger.error("方法xxx出錯～"); 複制
- 日志列印可以合并的合起來列印
#合并前slogger.info("name={}", name);logger.info("type={}", type);logger.info("timeCosted={} ms", timeCosted);
#合并後logger.info("name={},type={},timeCosted={}", name,type,timeCosted); 複制
- 日志是否有用MDC，如有需要檢查是否合理的調用了remove / clear方法，防止線程間日志列印幹擾等問題。
- ... ...
經驗性檢查
- SimpleDateFormat是否被定義成一個全局變量，如下代碼，多線程将出現問題。
private static final SimpleDateFormat DEFAULT_FOMAT = new SimpleDateFormat("yyyy-MM-dd");
public static Date formatDefaultDate(String date) { try { return DEFAULT_FOMAT.parse(date); } catch (ParseException e) { return null; } } 複制
SimpleDateFormat更多的資訊請參考《SimpleDateFormat線程不安全示例及其解決方法》
- 金額處理是否使用BigDecimal類型，不能使用float和doube。
另外，BigDecimal對象建立，如果沒有使用好，也可能出現問題。比如下面的示例：
//0.299999999999999988897769753748434595763683319091796875 BigDecimal v1 = new BigDecimal(0.3d); System.out.println(v1);
//0.3 BigDecimal v2 = new BigDecimal("0.3"); System.out.println(v2);
//0.3 BigDecimal v3 = BigDecimal.valueOf(0.3d); System.out.println(v3); 複制
- 檔案流使用後是否正常關閉。我們需要finally關閉流，以防止記憶體洩漏。
- ... ...
逾時問題
- 新增的服務，如Dubbo服務，需要有逾時設定
- 分布式鎖需要有逾時處理
- 調用下遊接口或者調用各種中間件需要有逾時的機制
- ... ...
代碼職責和調用關系
- 代碼職責是否清晰，比如UserService寫了很多權限資源相關的邏輯處理方法就不好。
- 層次調用是否正确，一般Controller -> Service -> DAO 。如果Controller -> DAO就不好
-
Dubbo服務不要調用調用自己的服務。
這個怎麼了解呢？比如有一個UserRpcService，其有update和query的接口，假設，update裡面需要做使用者查詢的操作做判斷，剛好query能夠滿足。
public class UserRpcServiceImpl implements UserRpcService {
public int update(xxx) {
//調用query，而query也是一個rpc接口 query(xxx); }
public int query(xxx) {
}
} 複制
不建議在rpc接口，再調用自身的rpc接口，如本示例的update和query。
- ... ...
線程池建立線程
- 對一些多線程邏輯，使用線程池建立線程，而不是通過new Thread等方式建立。
- ... ...
進階篇
資料庫方面檢查
- 對表增加字段Alter table xxx add column xxxx ... ，需要檢查下目前表中的記錄數，如果資料量很大這個是不能做的。需要同DBA進行溝通。或者自己通過建一個新表（比原表多一個字段），用操作新表替換舊表來完成。這裡需要完成原表資料遷移等其他内容。
- 索引添加是否合适
- 是否存在危險SQL，如update / delete 語句中的變量是否在業務能夠保證有必要的值，不能出現很多值沒有，導緻if test 都不滿足，導緻更新的範圍擴大。
<if test="xxx !=null "> 複制
- 檢視是否有循環單個插入記錄的情況，改成批量插入。
- ... ...
安全滲透方面檢查
- 檔案上傳是不是隻判斷了檔案字尾？ 隻判斷字尾，攻擊方可以将一個jsp等檔案僞裝成jpg等格式的檔案，進而成功上傳到服務，導緻伺服器資訊洩漏。
- 短信驗證碼對一個手機号，是否調用接口就能給使用者發一個新的短信驗證碼，進而造成短信轟炸？我們需要在短信産生的有效期内保證不重新産生短信驗證碼。
- 接口是否存在越權檢視等風險？比如A可以通過id檢視屬于B的裝置資訊？
- ... ...
接口保護檢查
- 清單查詢是否有pageSize的限制（如最多一次查詢100條）。如果不限制，那麼假設pageSize可以為5000條，那真的是簡直了，對吧？
- 如果接口調用需要有次數限制，我們還要考慮是否對方法等有限流的措施？
- ... ...
模式應用
- 如果使用了D.C.L（Double-Check Lock），那麼看是否有volatile修飾
- 抽象是否充分？比如，有不同類型的服務接口調用，主要有如下幾個步驟：
1、參數校驗2、檢查是否有流量3、執行業務邏輯4、記錄調用日志5、流量扣減6、... ... 複制
如果每個服務都自己寫一遍，不是很合适，也不不容易維護和擴充。在這種情況下，不同的服務調用主要是步驟#1和步驟#3有個性化的處理，可以抽象出來。
比如寫一個簡單的模版，步驟#1和步驟#3使用abstract方法，由子類具體實作。
抽象類
public abstract class AbstractServiceHandler<T extends ServiceRequest> {
/** * 模版方法 */ public void handle(T param) { // 1、驗證 this.doValidate(param);
// 2、校驗是否有流量
// 3、執行業務邏輯 doBusiness(param);
// 4、記錄調用日志 // 5、流量扣減 // 6、 ... ... }
// 子類做校驗 protected abstract void doValidate(T param);
// 子類做業務邏輯 protected abstract void doBusiness(T param);
} 複制
服務請求參數
import java.io.Serializable;
public class ServiceRequest implements Serializable {
private static final long serialVersionUID = 4314529075012784996L;
// 屬性省略
} 複制
決策流服務處理類
public class DecisionFlowHandler extends AbstractServiceHandler<ServiceRequest> {
@Override protected void doValidate(ServiceRequest param) { // TODO Auto-generated method stub
}
@Override protected void doBusiness(ServiceRequest param) { // TODO Auto-generated method stub
}
} 複制
決策工具服務處理類
public class DecisionToolHandler extends AbstractServiceHandler<ServiceRequest> {
@Override protected void doValidate(ServiceRequest param) { // TODO Auto-generated method stub
}
@Override protected void doBusiness(ServiceRequest param) { // TODO Auto-generated method stub
}
} 複制
- 可以考慮一些設計原則，比如單一職責/接口隔離等。
可以參考《設計模式幾大原則》
- ... ...
高階篇
在這個篇章部分，我們要對一些“失敗”的場景，方案&應急有一定的考慮。
重試和幂等
- 針對系統間的資料同步，如果失敗？我們是否有重試機制？
- 針對計費等場景，失敗後重試調用，我們的接口是否支援幂等？
- 檔案導入任務，如中斷，我們是否有重新開機任務的機制，繼續完成？
- ... ...
方案和應急
- 緩存對象增加屬性（比如User加一個type），釋出上線的時候，緩存的Key是否有做版本調整。如果更新後Key不變，可能導緻Redis的value是由原服務更新，導緻新改的内容上線後，可能還是取的原來的值（不包含type）。
- 比如在某些場景中，Redis緩存添加是不是有開關（一般由配置中心推送設定），以防止在緩存不是很正确的場景下，用資料庫來保底
- 比如涉及資料遷移或者Redis叢集更新（由5.0改成6.0）, 切流的計劃是否合理？
- 有時候為了減少RPC調用或者少走網絡，會結合Redis（分布式） + Guava（本地緩存）結合使用，本地緩存更新的政策是什麼？（叢集下需要通過消息廣播來達到快速更新各機器本地緩存的目的）
- 緩存存放的值是否為大對象，緩存個數多大？失敗政策是什麼？緩存雪崩/并發等場景是否有考慮等等
- ... ...
更多緩存的一些知識，讀者可以從之前的文章《啊哈！緩存》了解更多内容。
小結
本文主要從關注代碼本身，對Code Review做了簡易的說明，想到啥就寫了點啥。
Code Review對代碼的關注點，遠不止這些，本文也算是一個簡單的抛磚引玉，有興趣的讀者可以一起留言探讨。