技术分享 | CodeReview主要Review什么?
源寶導讀:Code Review, 意即代碼審查,是指一種有意識和系統的召集其他程序員來檢查彼此的代碼是否有錯誤的地方. 在敏捷團隊中推行CodeReview, 可以幫助團隊快速成長.本文將分享在"天際-建模平臺"如何推行&實踐CodeReview。
一、為什么要 Code Review?
要不要Code Review,需要結合當前的環境和形勢來決定。如果項目組開發任務極其緊張,?此時再進行Code Review可能會收到不利的效果,因勢利導,求同存異是Code Review的核心所在。
Code Review, 需要和項目組成員溝通和配合,同時也在檢驗各成員的技術水平。
需要注意的是,人都有惰性,每個人都有自己的一套行為準則和規范,要想把他們的行為或規范統一起來,很容易使他們產生抗拒心理。
在Code Review之前,和項目成員溝通好,有一個共同的愿景,并輔助以相應的培訓,把部分人員的短板補齊。會減輕項目成員的一些不適感,也會使得項目運行更加順暢。
同時, 也要注意,不要事事求完美主義,一步求成。"慢即是快"。
二、?Code Review的前提條件
Code Review本身屬于"事后"工作,可以起到查漏被缺的作用。?
在推行Code Review時,需要先提前準備以下幾個工作,以便團隊能夠更快更好的接受和實施。
建立規范, 包含:
編碼規范,如變量命名,文件命名規范等。
設計原則,如單一職責原則,最少知識原則等。
分支管理策略。
完善的文檔,方便查閱。文檔內容最好能共建,千萬不可出現"一言堂";
制定Code Review流程&目標,以及實施周期。
例如, 根據當前團隊的實際情況,將Code Review實施分為3個階段。
第一階段,重點關注,恰到好處的函數注釋,"硬編碼"問題,常見變量命名規則等,預期實施周期為1~3個月。
第二階段,重點關注,代碼耦合性,單一職責、最少知識原則, 潛在隱患,性能問題等,預期實施周期為3~6個月。
第三階段,重點關注,模塊實現方案,設計模式,最佳實踐,代碼重構等。
在實施過程中,如果遇到比較大的阻力或困難,推行Code Review所得到的收益較低時,可以考慮適當"休息"一段時間。
三、常見的Code Review項
3.1 git 提交規范
git commit提交規范, 如:
不標注信息
不及時commit
提交時標注的信息不規范等
都是嚴重阻擾review的因素之一,除了常規的描述信息外,還可以按類型等備注:
feat: 新特性
fix: 修改問題
refactor: 代碼重構
docs: 文檔修改
chore: 其他修改
test: 測試用例修改
style: 代碼格式修改等等
當然,也要以利用一些工具,來協助我們完成基本的約束和檢查。
3.2 風格
3.2.1 可讀性
衡量可讀性,有很好的實踐標準,即Code Review時能否非常容易的理解代碼邏輯,如果不能,那意味著代碼的可讀性要進行改進。
3.2.2 命名
命名對可讀性非常重要,我個人傾向于函數名/類名長一點都沒關系,但必須能清晰的表明函數/類的作用。
英語用詞盡量準確, 哪怕需要借助翻譯工具,也是值得的。但有一點需要注意,如果使用的單詞很冷門,都沒人認識, 那就不要使用。
3.2.3 函數體長度/類長度
函數體太長,不好閱讀,一般建議不要超過50行。
類太長, 如超過1000行,那可能就要看下是否違反了"單一職責"原則。
3.2.4 參數個數
不要太多,一般不要超過5個, 超過5個,建議使用對象。
3.2.5 注釋
恰到好處的注釋,能夠幫助我們理解函數/類的作用。
3.3 架構/設計
3.3.1 單一職責原則
這是經常被違背的原則,也是最難運用好的原則。
一個類只做一類相關的事情。
一個函數/方法, 最好只做一件事情。
3.3.2 行為是否統一
什么是行為統一? 例如:
錯誤處理是否統一
錯誤提示是否統一
彈出框是否統一
......
同一邏輯/行為,有沒有執行同樣的代碼路徑,?低質量的代碼一個特征是,同一邏輯/行為,在不同的地方或不同的方式觸發時,沒有執行同樣的代碼路徑(產生出不同的結果),或者是各處copy一份實現,導致非常難以維護.。
3.3.3 代碼污染
代碼有沒有對其他模塊強耦合。
3.3.4 重復代碼
主要看有沒有把公用組件,可復用的代碼、函數抽取出來。
3.3.5 ?開放-封閉原則
簡單理解是,看代碼好不好擴展。
3.3.6 健壯性
核心數據有沒有強制校驗?
對業務有沒有考慮完整, 邏輯是否健壯。
有沒有潛在的bug?
有沒有內存泄露?有沒有循環依賴?
......
3.3.7 錯誤處理
有沒有很好的Error Handling? 如網絡出錯,,IO出錯等。
3.3.8 面向接口編程/面向對象接口編程
主要看有沒有進行合適的抽象,把一些行為抽象為接口。
3.3.9 效率/性能
客戶端程序對頻繁的消息和較大數據等耗時操作是否處理得當。
關鍵算法的時間復雜度是多少? 有沒有潛在的性能瓶頸?
3.3.10 代碼重構
新的改動是打補丁,讓代碼質量繼續惡化,還是對代碼質量提升有幫助?
四、Code Review如何實施?
結合當前建模小組團隊的實際情況:
新人較多,多對業務不對熟悉。
前期團隊人員少,但有CodeReview的文化。
已有相關的編碼規范的知識沉淀,但因之前迭代任務重/人力資源緊張等因素,?導致未長期推行下來。
針對以上情況,經過團隊內部協商研討,輸出相應的解決機制:
"結對編程",每個Story盡量安排兩位同學一起完成(1位主責人),主要有兩個目的:
老帶新,在工作實踐中多溝通交流,幫助新員工盡快熟悉業務,融入團隊。
相互審查代碼,提高代碼質量,同時促進新老員工溝通交流,思想上多碰撞。
兩級代碼審查機制
第一級,由"結對編程"小組的兩位同學交叉審查代碼,過濾掉一些簡單的代碼問題。
第二級,由技術leader進行最終審查,確保代碼審查保質保量完成。
定期分享收錄的典型問題
代碼審查過程中,發現的一些典型問題,及時收錄并做好分析。
定期開展"壞代碼"分享會。
主要實施分三個階段進行:
第一階段, 恰到好處的函數注釋, "硬編碼"問題,常見變量命名規則等,預期實施周期為1~3個月。
第二階段,代碼耦合性, 單一職責、最少知識原則, 潛在隱患,性能問題等,?預期實施周期為3~6個月。
第三階段,模塊實現方案,設計模式,最佳實踐,代碼重構等。
從目前實施過程的體會, 以及結合同學們的一些經驗來看,在Code Review時建議:
單次查看代碼不多于500行,人的精力有限,一次審查太多的代碼,收益可能不理想。
單次審查建議不要超過30分鐘。
祖傳代碼怎么辦?
只查看git diff中, 帶有"+"的代碼,歷史代碼可以暫時忽略不看,在不斷的迭代中,遲早會遇到需要修改"祖傳"代碼的。
這也是一種"妥協"的辦法,讓代碼審查者和開發者之間有一個平衡。
對于"開發者"的心理會產生一定的影響,他/她可能需要提前做好準備,萬一哪天需要修改"祖傳"代碼呢?
在下一節中,收錄了實施過程中發現的一些典型的問題代碼,大家可以一起"品嘗"下。
五、?低質量代碼的常見特征
5.1 案例1
違反"單一職責"原則。
同一邏輯/行為, 通過不同的方式觸發時, 不會執行同樣的代碼路徑。
缺少注釋。
函數/類命名亂, 詞不達意, 或"掛羊頭賣狗肉"。
上例函數,其實是想對傳入的參數進行判斷,如果值無效,取默認圖片,否則返回拼接地址好的img src相對地址。從函數的實現來看, 有如下幾個問題:
函數名setDefaultImg并不合適,無任何對數據的更新/修改,最終目的是想拿到img的拼接地址(或者默認圖片)。
形參`res`是一個對象, 實際使用到的屬性只有兩個基本類型,遵循最少知識原則,應修改形參。
`let imgUrl`,其實是拼接url的前綴,取名如 `imgPrefixUrl`會比較合適。
let 使用不當,如 `let imgUrl`,因涉及不到變量再賦值,使用`const是比較合適的`。
`res.imgId`,即異常情況的處理,少了對`res.type`的處理。
變量命名,如`previewImgSrc`,取此名也不太合適,該src使用的目的并不確定,建議使用有比較通用涵義的名字。
可考慮重構為:
export function getImgSrc(imgId, imgType) { if(typeof imgId !== 'string' || typeof imgType !== 'string') { return require('@/asserts/images/icon.png') } return `${getImagePrefixUrl()}${imgId}.${imgType}?t=${+new Date()}`}5.2 案例2
參數傳遞自由度過大,易引發后續不可控的風險。
上例代碼中,傳遞給`$emit`的數據是`this.$props`,會有以下問題:
存在高耦合關系( 數據結構 耦合)。
接收參數,建議盡量使用:基礎數據類型,如:字符串, 數字,布爾值等,非必要情況下, 不使用復雜數據類型,如: Array, Object等。
可考慮重構為:?
handleSelect(){ this.$emit('select', this.$props.userId)}5.3 案例3
違反"單一職責"原則,一個函數處理多個任務,可能會在調函數時產生“異外的感覺”。
形參設計過度(形參無須解構,后續若有需要添加新的參數時再添加,也可考慮使用options參數等來實現擴展)。
從上面的代碼中,我們可考慮從以下幾個角度著手重構, 以增強可讀性。
在入口函數處理,調用功能函數(通過函數名來區分功能),明示調用邏輯。
減少代碼量,增強可讀性,能用ES6 方法解決的就用它解決,盡量不造不必要的輪子,比如使用Array.findIndex來解決查找數組是否存在某一項的問題。
優化業務邏輯,刪除不必要的代碼,增強代碼的可維護性。
函數重新命名,務必做到“見名知意”。
添加必要的注釋(遵循jsdocs注釋規范)。
可考慮重構為:
mounted(){const {row} = this.getSelectedRow();this.deleteRow(row);this.resetGrid();},/*** @description: 刪除指定的表格行數據* @param {Object} row, 表格行數據* @return*/deleteRow(row) {this.$refs.grid.remove(row)}/*** @description: 重置表格**/resetGrid(){if(this.$refs.grid.innerData.length && this.$refs.grid.innerData.findIndex(item => item.isQueryField) < -1){this.$refs.grid.innerData[0].isQueryField = truethis.handleChange({name: 'isQueryField',row: { $index: 0 },data: this.$refs.grid.innerData})}}六、總結
敏捷團隊由"創建-振蕩-成熟-規范"的過程中, 不同的階段建議采取不同的方式開展CodeReview, 需要考慮團隊的整體情況以及當前公司的業務的緊急程度,?優先解決首要問題,再接著解決其他次要問題。
通過幾個月的漸進 & 持續的推行Code Review后,團隊成員是可以達到以下效果的:
促進團隊成員的相互交流, 加速團隊朝"成熟-規范"階段前進。
知識分享, 促進團隊成員進行反思和總結。
代碼質量和可維護性, 可讀性等的提高。
查漏補缺, 發現一些潛在的問題點等。
最佳實踐, 能夠更好更快的完成任務的方法。
------ END ------
作者簡介
張同學:?開發工程師,目前負責天際-建模平臺產品的研發工作。
也許您還想看:
技術分享|Java SDK動態數據源和上下文機制
技術分享|NodeJS分布式鏈路追蹤實現
技術分享 | Java SDK 元數據驅動的事件通信架構
技術分享|Hangfire深度實踐
技術分享 | APT結合JavaPoet生成模板化Java源代碼文件
技術分享 | 玩轉高效UI自動化測試
更多明源云·天際開放平臺場景案例與開發小知識,可以關注明源云天際開發者社區公眾號:
【建模】文檔服務提供高保真打印模式
明源云·天際硬核技術認可:獲華為鯤鵬技術認證書
天際·開發者社區“重裝發布”!
總結
以上是生活随笔為你收集整理的技术分享 | CodeReview主要Review什么?的全部內容,希望文章能夠幫你解決所遇到的問題。
- 上一篇: 一行代码完成定时任务调度,基于Quart
- 下一篇: 探索 dotnet core 为何在 W