翻譯自
https://medium.com/@catalinaturlea/how-i-do-code-review-caa0e5828d8e

作者 Catalina Turlea

我的codereview習(xí)慣 的清單

就像我上篇文章寫的那樣，我真的相信 code reviews 是一個(gè)非常有價(jià)值的習(xí)慣。

我是從一個(gè)非常有才華并且也很嚴(yán)厲的開(kāi)發(fā)者那里學(xué)習(xí)了一些關(guān)于code reviews的工作。開(kāi)始總是困難的，但是它總是在我意想不到的地方幫我提高自己的能力。

至那以后，我花費(fèi)了一些時(shí)間和精力去進(jìn)行適當(dāng)?shù)?reviews。我的同事都知道，在進(jìn)行reviews的時(shí)候，我是最容易激動(dòng)的人，而且在那個(gè)時(shí)候，我也會(huì)很嚴(yán)厲。

以我的經(jīng)驗(yàn)來(lái)說(shuō)，同樣的事情，總是會(huì)反復(fù)又反復(fù)，所以我學(xué)會(huì)總結(jié)這類事情。我是一個(gè)iOS 開(kāi)發(fā)者，所以我想到的大多和iOS平臺(tái)相關(guān)。

但是我認(rèn)為，不論是哪種開(kāi)發(fā)語(yǔ)言，這些東西應(yīng)該是相通的。

下面的代碼段是基于Swift和Objective-C的，但是我希望對(duì)于其他語(yǔ)言的開(kāi)發(fā)者來(lái)說(shuō)也同樣清晰。So，讓我們開(kāi)始review 代碼吧。

Language independent red flags

1、Don’t repeat yourself -DRY （不要重復(fù)）

類似下面的代碼

// 檢測(cè) 數(shù)組是否為空
if array.count == 1 {
   submitFormWithValues("Only one value selected", array)
} else {
   submitFormWithValues("\(array.count) values selected", array)
}

第一眼看上去好像沒(méi)什么問(wèn)題，但是如果你仔細(xì)看，你會(huì)發(fā)現(xiàn)，這個(gè)方法會(huì)調(diào)用兩次。即使他只有一個(gè)判斷條件。
假如，舉個(gè)例子，這個(gè)方法如果被重命名，或者添加第三個(gè)參數(shù)，你需要在兩個(gè)位置修改代碼，因?yàn)檫@個(gè)是你代碼中的重復(fù)部分。

下面的代碼就會(huì)很好的避免這個(gè)問(wèn)題

let string = array.count == 1 ? "Only one value selected" : "\(array.count) values selected"
// 現(xiàn)在我們就只會(huì)調(diào)用一次這個(gè)方法了
submitFormWithValues(string, array)

現(xiàn)在，如果你要袖肥這個(gè)方法的話，你只需要修改一個(gè)地方就可以了，在也不需要重復(fù)了。

這還有很多類似的例子。當(dāng)你在一個(gè)方法或者一個(gè)文件中看到類似的代碼時(shí)，你需要問(wèn)問(wèn)自己，為什么不把重復(fù)的代碼刪掉？

2、Returning booleans

func isEmpty(array: [String]) -> Bool {
   if array.count == 0 {
       return true
   } else {
       return false
   }
}

每當(dāng)你返回一個(gè)布爾值前，你應(yīng)該仔細(xì)看下你的代碼，因?yàn)榇蠖鄶?shù)情況下，你都可以用一種更簡(jiǎn)單的寫法

func isEmpty(array: [String]) -> Bool {
   return array.count == 0
}

這種寫法同樣適用于 計(jì)算對(duì)象的布爾屬性。就像這樣：

func hideHeaderIfEmptyTableView() {
   if datasource.count == 0 {
       header.hidden = true
   } else {
       header.hidden = false
   }
}

這是非常相似的例子，當(dāng)你賦值布爾屬性或者返回布爾屬性的時(shí)候，你可以使用下面的這種更直接的寫法

func hideHeaderIfEmptyTableView() {
   header.hidden = (datasource.count == 0)
}

3、Reduce the level of indentation and use early exits

在你的代碼中嵌套的層級(jí)越多，代碼越難被閱讀和理解。我們的大腦一次只能處理有限的狀況，所以，越多的循環(huán)，就會(huì)讓我們的大腦越有負(fù)擔(dān)。

我們不應(yīng)該將所有的包含在一個(gè)if循環(huán)中，你只需要添加一個(gè)不循環(huán)的條件在前面，并且return它

所以，這是一個(gè)比較直接的途徑，而不是想下面這樣：

func myMethod(elements: [String]) {
   if elements.count > 0 {
      for element in elements {
        // do some processing
      }
   }
}

你應(yīng)該使用：

func myMethod(elements: [String]) {
   if elements.isEmpty {
      return
   }
   for element in elements {
      // do some processing
   }
}

我也意識(shí)到它可能只是代碼風(fēng)格的問(wèn)題，但是我認(rèn)為這樣做更容易去遵守，并且讓控制流更加的線性。

4、“Shy” code

就像The Pragmatic Programmer中提到的那樣，你的代碼應(yīng)該永遠(yuǎn)是“shy code”。
盡量在最小范圍內(nèi)寫每件事，只有在你真的需要的時(shí)候增加這個(gè)范圍。
舉個(gè)例子，從所有事情** **私有開(kāi)始，包括你的屬性和方法。

我認(rèn)為這同樣適用于屬性的暴露級(jí)別，假如你有一個(gè)內(nèi)部包含一個(gè)圖片的自定義UI組件，同時(shí)它需要在外部設(shè)置，你可以提供一個(gè)更新它的方法，而不是將它作為一個(gè)可讀寫的屬性。這樣iamge就會(huì)作為一個(gè)只讀的存在，并且你也可以在你屬性的值上有更多的限制。

5、Remove empty methods and any unused generated code

The best code is the one that does not exist

要養(yǎng)成將不用的代碼刪除的習(xí)慣（哪怕是IDE自動(dòng)生成的）。不要讓你的代碼中包含空方法，未用的變量，導(dǎo)入，無(wú)效的注釋。如果你不需要它，那就刪掉它。

刪除代碼應(yīng)該是開(kāi)發(fā)者最喜歡的活動(dòng)之一。

一些其他的建議

當(dāng)你review代碼時(shí)，你應(yīng)該牢牢的記住這些：

• Hard-coded values：首先，你應(yīng)該問(wèn)自己，沒(méi)有方法去替代這個(gè)靜態(tài)值么？如果有，那么讓它變得更好，如果沒(méi)有，那就將它存儲(chǔ)在靜態(tài)變量中。這些值的命名應(yīng)該和上下文有所聯(lián)系，比如imageDefaultWidth 或者 controllerIdentifier，在有利于在一段時(shí)間后，容易理解這些值的意思。這樣做的另一個(gè)好處是，這些值會(huì)在不同的地方重復(fù)出現(xiàn)。如果沒(méi)有將它存在變量中，那么當(dāng)你要做出改變的時(shí)候，比如將值從3修改為4，那么你將要在任何需要改變的地方做出修改。
  建立團(tuán)隊(duì)標(biāo)準(zhǔn)和準(zhǔn)則 - 確定標(biāo)準(zhǔn)，并且每個(gè)人都要遵守，不要在code review*時(shí)忽略這點(diǎn)。它應(yīng)該包括變量的命名規(guī)則，代碼規(guī)范，團(tuán)隊(duì)公認(rèn)的開(kāi)發(fā)語(yǔ)言好的嘗試。

• Log your errors - 或許這點(diǎn)是顯而易見(jiàn)的，但是我還是認(rèn)為，它值得提起。確保每次你的代碼出現(xiàn)問(wèn)題時(shí)都能及時(shí)記錄，哪怕你認(rèn)為它不可能發(fā)生，記下這個(gè)錯(cuò)誤盡可能多的信息，它非常有用。

Objetive-c / swift specific observations

1、確保已經(jīng)注銷了觀察者

每次review或者編寫有關(guān)于注冊(cè)某種通知或者訂閱時(shí)，你都應(yīng)該確認(rèn)你是否注銷或者取消訂閱了。如果你忽略了這點(diǎn)，我相信你會(huì)知道“Message sent to deallocated instance”有多難debug。

我十分強(qiáng)調(diào)這一點(diǎn)，check，check，check。

2、Most specific method arguments

舉個(gè)例子，button callbacks。應(yīng)當(dāng)為方法參數(shù)添加最詳細(xì)的類型和調(diào)用對(duì)象。

好處體現(xiàn)在兩個(gè)地方：一旦它更清晰，那么只會(huì)有一個(gè)button能調(diào)用這個(gè)方法，你不需要去檢查對(duì)象的屬性，不用描述它或者任何事情。你也許現(xiàn)在不需要，但是你需要為將來(lái)的開(kāi)發(fā)流出空間。

@IBAction func didClickContinue(sender: id) { 
   // 一年后，估計(jì)你就記不起調(diào)用這個(gè)方法的是哪種類型的對(duì)象了
}

你應(yīng)該用一些更加有意義的描述去快速的指出這段代碼的實(shí)際用途

// 怎樣使用這個(gè)方法？你會(huì)知道這個(gè)方法怎樣用，也會(huì)知道哪個(gè)對(duì)象對(duì)調(diào)用這個(gè)方法
@IBAction func didClickContinueButton(sender: UIButton) {
}

關(guān)于方法：

// 永遠(yuǎn)都要將調(diào)用對(duì)象包含在callback中
let tapGesture = UITapGestureRecognizer(target: self, action: “didTap:”)

3、Most generic object references

當(dāng)你使用一個(gè)對(duì)象時(shí)，盡可能的將它存在最通用的類型中，這不影響你使用你需要的功能

例如：

private var viewController: AlertViewController?
init() {
   viewController = AlertViewController()
   navigationController?.pushViewController(viewController,     animated: true)
}

你需要注意的是，push方法只需要一個(gè)UIViewController而不是一個(gè)AlertViewController，所以你應(yīng)該將它存在一個(gè)最通用的類型中比如UIViewController。

// 我們不需要AlertViewController的特殊屬性，所以，我們將它存為*UIViewController*
private var viewController: UIViewController?
init() {
   viewController = AlertViewController()
   navigationController?.pushViewController(viewController, animated: true)
}

這里的想法是，不要在沒(méi)有給出具體需求的時(shí)候，給出信息。* *對(duì)象之間聯(lián)系越少，依賴就越小。

4、Documentation - Pargma marks

當(dāng)一個(gè)類變得越來(lái)越大的時(shí)候（無(wú)論多么優(yōu)秀的開(kāi)發(fā)者，都會(huì)出現(xiàn)這樣的問(wèn)題），為public/private methods, the protocol implementations, public/private variables做一些限定，對(duì)于我們理解和工作都是非常有用的。

Other things to keep an eye for

• 循環(huán)引用 - 保證delegate是弱引用。同樣在blocks中保證self 是weak，去避免循環(huán)引用。
• 不必要的代碼 - 就像前面提到的，不要在你的代碼中包含無(wú)用的代碼。