臨近過(guò)年的日子,是個(gè)回顧的好機(jī)會(huì)。
正好前一段看到這篇翻譯介紹 讀自己以前代碼的Kata食棕,拿來(lái)練習(xí)練習(xí)簿晓。
其實(shí)Thomas在提出Kata概念的時(shí)候千埃,涵蓋的范圍是遠(yuǎn)大于編碼層面的放可。只是由于其它類型的Kata的判定標(biāo)準(zhǔn)比較“虛”耀里,所以不常拿來(lái)練習(xí)。
這個(gè)Kata的關(guān)注點(diǎn)在于刻意練習(xí)閱讀代碼底哥,而且是自己寫的代碼房官,來(lái)體會(huì)代碼中各種細(xì)節(jié)的長(zhǎng)期影響翰守。通過(guò)主動(dòng)的回顧來(lái)形成良好習(xí)慣,并增強(qiáng)review能力拒课。
我自詡是個(gè)“代碼考古學(xué)家”早像,常常修一個(gè)bug把80%的時(shí)間用在了翻歷史記錄,來(lái)琢磨它為啥能寫成這個(gè)樣子臀脏。同時(shí)也是個(gè)熱衷于review別人代碼的人揉稚,不憚?dòng)趯?duì)別人的代碼說(shuō)三道四熬粗,想必很惹人厭了驻呐。
這次算是找個(gè)機(jī)會(huì)把自己的代碼擺出來(lái)灌诅,自食其果吧。
代碼的選擇
Kata的要求是寫了超過(guò)一年含末,記憶有些模糊猜拾。代碼量在500至1000行。
我選擇了一個(gè)小項(xiàng)目佣盒,小到基本由我一個(gè)人開(kāi)發(fā)挎袜。而且除了對(duì)口的產(chǎn)品經(jīng)理,也再?zèng)]其他人關(guān)心了肥惭。所以當(dāng)初可以隨心所欲的搞盯仪。
然而即便這么小的項(xiàng)目务豺,做完以后客戶卻要求與最初商定的需求文檔幾乎完全不一樣的功能磨总。最終,所有的業(yè)務(wù)對(duì)象推倒重新做了一遍笼沥。所以也可以算是個(gè)典型的軟件項(xiàng)目吧蚪燕。
基本上是用outside-in 式TDD開(kāi)發(fā),雖然當(dāng)時(shí)還不是特別自覺(jué)的采用這種流程奔浅。
從中選擇了一個(gè)類來(lái)做Review馆纳。選擇時(shí)參考了《修改代碼的藝術(shù)》作者關(guān)于技術(shù)債分析的建議:一個(gè)類修改越頻繁,說(shuō)明它越可能將來(lái)被改動(dòng)汹桦,越可能存在技術(shù)債鲁驶。
我統(tǒng)計(jì)了所有的提交記錄,選出一個(gè)提交次數(shù)最多的類舞骆,名叫WorkflowManager钥弯。和標(biāo)準(zhǔn)工作流沒(méi)什么關(guān)系径荔,是一個(gè)實(shí)現(xiàn)簡(jiǎn)單工作步驟流轉(zhuǎn)的類。
第一遍讀:假定這是大牛寫的代碼脆霎,記錄驚艷的地方
- 短总处。類只有200行,每個(gè)方法的長(zhǎng)度都少于30行睛蛛。
雖說(shuō)是個(gè)簡(jiǎn)單的流程控制鹦马,用語(yǔ)言來(lái)描述還真不是一兩句話能說(shuō)清的。相比起來(lái)代碼算是相當(dāng)簡(jiǎn)潔了忆肾。 - 一致荸频。代碼格式,命名規(guī)范客冈。常用寫法都很一致旭从。看起來(lái)很整齊场仲。
- 可讀遇绞。方法和變量都起了有意義的名字。
- 測(cè)試覆蓋燎窘。類共有15個(gè)方法,對(duì)應(yīng)的測(cè)試有19個(gè)用例蹄咖。測(cè)試代碼量是實(shí)現(xiàn)代碼的兩倍褐健。沒(méi)有專門用工具統(tǒng)計(jì)覆蓋率,按照從其它項(xiàng)目得到的經(jīng)驗(yàn)澜汤,應(yīng)該在90%以上蚜迅。
- 每個(gè)測(cè)試都使用了統(tǒng)一的
Given/When/Then
格式。 - 每個(gè)測(cè)試驗(yàn)證一個(gè)概念俊抵。對(duì)于相同方法不同使用情境的情況谁不,分別寫對(duì)應(yīng)的測(cè)試用例。
- 在測(cè)試中使用有意義的變量命名和assert message來(lái)說(shuō)明測(cè)試的意圖徽诲,比如:
//Given
User original = new User("assignee before operation");
workSet.assignee = original;
//When
manager.operate(workSet);
//Then
assertEquals("Not change assignee when operate work set", original, workSet.assignee);
第二遍讀:假定這是個(gè)二貨寫的代碼刹帕,記錄爛的地方
其實(shí)在看第一遍的時(shí)候就開(kāi)始忍不住想這些地方應(yīng)該寫的更好了。我果然是不善于發(fā)現(xiàn)優(yōu)點(diǎn)谎替,專愛(ài)挑毛病偷溺。
- 單一職責(zé)。
- 這個(gè)類叫做
WorkflowManager
, xxxManager這個(gè)名字暗示了職責(zé)不清钱贯。 - 果然挫掏,通過(guò)Clean Code中學(xué)到的辦法,統(tǒng)計(jì)每個(gè)方法與成員變量間的關(guān)系秩命。發(fā)現(xiàn)每個(gè)成員變量都只被很少幾個(gè)方法使用尉共。說(shuō)明這個(gè)類缺乏內(nèi)聚性褒傅,是多個(gè)職責(zé)的雜燴。
- 在測(cè)試?yán)镆灿畜w現(xiàn)袄友,某幾個(gè)用例關(guān)注于某個(gè)方法殿托。它們依賴共有的前置條件,驗(yàn)證相似的結(jié)果杠河÷刀看起來(lái)就像是Test類里內(nèi)嵌了一個(gè)小Test類。
- 這個(gè)類叫做
- 未使用的參數(shù)券敌。說(shuō)明當(dāng)初是覺(jué)得“將來(lái)會(huì)用到”加上的這個(gè)參數(shù)唾戚,而不是由測(cè)試驅(qū)動(dòng)產(chǎn)生的。
- 注釋待诅。實(shí)現(xiàn)和測(cè)試代碼中都有說(shuō)明意圖的注釋叹坦。應(yīng)該重構(gòu)代碼使注釋沒(méi)有必要出現(xiàn)。
- 系統(tǒng)時(shí)間卑雁。實(shí)現(xiàn)代碼里有最近更新日期的時(shí)間戳字段募书。測(cè)試中在操作完畢后,直接用當(dāng)前系統(tǒng)日期與之比較测蹲。這樣做有兩個(gè)問(wèn)題莹捡。一是依賴環(huán)境,另一個(gè)問(wèn)題是如果測(cè)試執(zhí)行和驗(yàn)證的兩個(gè)點(diǎn)恰好跨0點(diǎn)就會(huì)失敗扣甲。
- 返回
null
篮赢。由于依賴的對(duì)象有可能返回null
, 這段代碼里相當(dāng)一部分在處理這種情況琉挖,使得主線邏輯有些模糊启泣。可悲的是示辈,之后它又會(huì)返回null
值給外面…… - 命名寥茫。有些方法名用的是類似事件的名稱,比如
stateChanged
矾麻,而不是一個(gè)動(dòng)作纱耻。 - 異常處理。代碼里有一部分邏輯是處理配置信息異常情況的射富,這種情況下程序無(wú)法正常完成操作膝迎。然而處理的不夠統(tǒng)一,有些地方悄悄的返回
null
值胰耗,有些記錄了日志限次,但是沒(méi)有明確指出這是某項(xiàng)配置的問(wèn)題,以及這個(gè)問(wèn)題可能造成的后果。 - 測(cè)試中assert message很難說(shuō)它是描述了應(yīng)該的行為卖漫,還是失敗時(shí)候的錯(cuò)誤费尽。比如:
assertNull("no start date after approval", workSet.startDate);
到底是應(yīng)該有這個(gè)日期還是沒(méi)有呢, - 有些message在程序行為變化后沒(méi)有隨之修改羊始。
- 每個(gè)test幾乎都有重復(fù)的mock權(quán)限相關(guān)數(shù)據(jù)的代碼旱幼,模糊了當(dāng)前測(cè)試的焦點(diǎn)
上面這些嚴(yán)格來(lái)說(shuō),應(yīng)該算是可以改進(jìn)的地方突委,還稱不上是二柏卤。不過(guò)下面這段就……
public List<Item> setSelectedItems(List<Item> items, WorkSet workSet) {
List<Item> remainItems = new ArrayList<Item>(items);
nextInputItem:
for (Item inputItem : items) {
for (Item item : workSet.items) {
if (item.key.equals(inputItem.key)) {
item.selected = inputItem.selected;
remainItems.remove(inputItem);
continue nextInputItem;
}
}
if (!inputItem.selected) {
remainItems.remove(inputItem);
}
}
return remainItems;
}
兩層for嵌套,不但嵌套for匀油,里面還套if缘缚,不但if,還continue敌蚜,不但continue桥滨,還從內(nèi)層continue到外層……
感覺(jué)差不多把今天我認(rèn)為不好的風(fēng)格全拿出來(lái)演練了一遍。瞬間有點(diǎn)“這是我寫出來(lái)的么弛车?”的感覺(jué)齐媒。不過(guò)稍稍回憶一下,就想起來(lái)我當(dāng)時(shí)還去專門查了查continue到外層的寫法纷跛。
你能看出來(lái)這是在干嘛么喻括?
其實(shí)是數(shù)據(jù)庫(kù)里的workset保存有若干個(gè)項(xiàng)目,每個(gè)項(xiàng)目有選中狀態(tài)贫奠。每次頁(yè)面提交時(shí)候會(huì)返回這些項(xiàng)目通過(guò)頁(yè)面操作后的選中狀態(tài)双妨。需要更新到數(shù)據(jù)庫(kù)中。
而且頁(yè)面還有可能增加原來(lái)數(shù)據(jù)庫(kù)里沒(méi)有項(xiàng)目叮阅,這些項(xiàng)目要在數(shù)據(jù)庫(kù)新建項(xiàng)目記錄,并且和workset關(guān)聯(lián)起來(lái)泣特。
這個(gè)雙層循環(huán)做了兩件事
- 對(duì)于已經(jīng)有的項(xiàng)目浩姥,按照頁(yè)面?zhèn)魅氲牧斜砀逻x中狀態(tài)
- 對(duì)于沒(méi)有的項(xiàng)目,放在另一個(gè)列表中作為返回值状您,方便給下一步做創(chuàng)建項(xiàng)目等操作勒叠。
如果讓你來(lái)重構(gòu)它,你會(huì)怎么做呢膏孟?
第三遍讀:假定這段代碼有個(gè)嚴(yán)重Bug眯分,今天找不出來(lái)你就完了。記錄找到的Bug
雖然沒(méi)有專人QC柒桑,也完全沒(méi)有測(cè)試階段弊决,但是因?yàn)橛芍倚宰訉懥艘欢褑卧獪y(cè)試和集成測(cè)試。每次本地保存代碼的時(shí)候都會(huì)執(zhí)行一遍。因而我對(duì)項(xiàng)目的質(zhì)量是相當(dāng)自信的飘诗。
最直觀的感受是在客戶驗(yàn)收演示會(huì)上与倡。
當(dāng)初開(kāi)發(fā)完成后扔在一邊,拖了半年多客戶才驗(yàn)收昆稿。已經(jīng)記不清具體代碼細(xì)節(jié)了纺座。客戶在操作演示過(guò)程中溉潭,走到一步走不下去了净响。當(dāng)時(shí)我心情毫無(wú)波動(dòng),一點(diǎn)也沒(méi)打開(kāi)源代碼檢查的沖動(dòng)喳瓣。
果然馋贤,稍稍回憶了一下就發(fā)現(xiàn)不是bug,而是操作失誤夫椭。
所以一開(kāi)始我認(rèn)為是沒(méi)法完成這一輪的目標(biāo)的掸掸。準(zhǔn)備能找到個(gè)錯(cuò)誤處理或者很特殊情況下的缺陷就交差吧。
而且讀代碼找bug真的好難蹭秋,要完完全全讀懂每一個(gè)細(xì)節(jié)在做什么扰付。這時(shí)候真巴不得當(dāng)初代碼寫的更好讀一點(diǎn)就好了。
沒(méi)想到仁讨,真找到了一個(gè)
public void statedChanged(WorkSetState originalState, WorkSet workSet) {
WorkPackage workPackage = workSet.workPackage;
if (isApproved(workSet)) {
/* ... */
} else {
for (WorkSet sibling : workPackage.workSets) {
if (isPending(sibling)) break;
workPackage.status = WorkPackageStatus.INPROGRESS;
}
}
workPackageRepository.save(workPackage);
}
這段代碼的本意羽莺,是一次審批計(jì)劃(WorkPackage
)中會(huì)分成多個(gè)分組(WorkSet
),有任何一個(gè)分組還未有開(kāi)始審批洞豁,那么整個(gè)計(jì)劃都處于未開(kāi)始狀態(tài)盐固。
然而真正實(shí)現(xiàn)的是,只要第一個(gè)分組是進(jìn)行中丈挟,整個(gè)計(jì)劃就會(huì)變?yōu)檫M(jìn)行中刁卜。如果第一個(gè)未開(kāi)始,則整個(gè)計(jì)劃沒(méi)有開(kāi)始曙咽。
相當(dāng)出乎意料竟然有這么明顯的一個(gè)錯(cuò)蛔趴。我反思了一下在重重測(cè)試包圍之下還漏掉這個(gè)Bug的原因。
- 有集成測(cè)試例朱,但是沒(méi)有驗(yàn)收測(cè)試孝情。沒(méi)有一個(gè)地方把業(yè)務(wù)流程的故事集中講一遍。因而在較大范圍的測(cè)試中漏掉了這個(gè)點(diǎn)。
- 單元測(cè)試中,僅僅構(gòu)造了一個(gè)計(jì)劃有兩個(gè)分組的情況蜀细,沒(méi)覆蓋到所有分支绍弟。
往深一層想,這個(gè)方法需要判定的情況比較多。所以針對(duì)它的測(cè)試用例本來(lái)就很多串慰,顯得在這個(gè)測(cè)試類中占了很大的比例隧熙。也就是在第二遍讀時(shí)發(fā)現(xiàn)的好像是內(nèi)嵌了一個(gè)小的Test類的情況婉弹。寫的時(shí)候心里就有種喧賓奪主的感覺(jué)睬魂,不由得懷疑是不是對(duì)這個(gè)方法過(guò)度測(cè)試了。進(jìn)而沒(méi)有仔細(xì)考慮來(lái)設(shè)計(jì)用例镀赌。
其實(shí)內(nèi)心產(chǎn)生不協(xié)調(diào)的真正的原因是實(shí)現(xiàn)類違反了單一職責(zé)氯哮,而不是測(cè)試重心失衡。
感受
這次練習(xí)的收獲收獲頗大商佛,雖然理論上自己有著對(duì)代碼質(zhì)量喉钢,可維護(hù)性,以及測(cè)試用例設(shè)計(jì)等方面的各種觀點(diǎn)良姆。但是這次時(shí)間膠囊式的體驗(yàn)肠虽,還是帶來(lái)了完全不同的心理感受和思索。
特別是第三輪找Bug的挑戰(zhàn)玛追。最后找到的時(shí)候是我記憶中找到Bug最開(kāi)心的一次税课。