（译）我的codereview习惯的清单

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

作者 Catalina Turlea

我的codereview习惯的清单

就像我上篇文章写的那样，我真的相信 code reviews 是一个非常有价值的习惯。

我是从一个非常有才华并且也很严厉的开发者那里学习了一些关于code reviews的工作。开始总是困难的，但是它总是在我意想不到的地方帮我提高自己的能力。

至那以后，我花费了一些时间和精力去进行适当的 reviews。我的同事都知道，在进行reviews的时候，我是最容易激动的人，而且在那个时候，我也会很严厉。

以我的经验来说，同样的事情，总是会反复又反复，所以我学会总结这类事情。我是一个iOS 开发者，所以我想到的大多和iOS平台相关。

但是我认为，不论是哪种开发语言，这些东西应该是相通的。

下面的代码段是基于Swift和Objective-C的，但是我希望对于其他语言的开发者来说也同样清晰。So，让我们开始review 代码吧。

Language independent red flags

1、Don’t repeat yourself -DRY （不要重复）

类似下面的代码

// 检测 数组是否为空
if array.count == 1 {
   submitFormWithValues("Only one value selected", array)
} else {
   submitFormWithValues("\(array.count) values selected", array)
}

第一眼看上去好像没什么问题，但是如果你仔细看，你会发现，这个方法会调用两次。即使他只有一个判断条件。
假如，举个例子，这个方法如果被重命名，或者添加第三个参数，你需要在两个位置修改代码，因为这个是你代码中的重复部分。

下面的代码就会很好的避免这个问题

let string = array.count == 1 ? "Only one value selected" : "\(array.count) values selected"
// 现在我们就只会调用一次这个方法了
submitFormWithValues(string, array)

现在，如果你要袖肥这个方法的话，你只需要修改一个地方就可以了，在也不需要重复了。

这还有很多类似的例子。当你在一个方法或者一个文件中看到类似的代码时，你需要问问自己，为什么不把重复的代码删掉？

2、Returning booleans


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

每当你返回一个布尔值前，你应该仔细看下你的代码，因为大多数情况下，你都可以用一种更简单的写法

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

这种写法同样适用于计算对象的布尔属性。就像这样：

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

这是非常相似的例子，当你赋值布尔属性或者返回布尔属性的时候，你可以使用下面的这种更直接的写法


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

3、Reduce the level of indentation and use early exits

在你的代码中嵌套的层级越多，代码越难被阅读和理解。我们的大脑一次只能处理有限的状况，所以，越多的循环，就会让我们的大脑越有负担。

我们不应该将所有的包含在一个if循环中，你只需要添加一个不循环的条件在前面，并且return它

所以，这是一个比较直接的途径，而不是想下面这样：

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

你应该使用：

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

我也意识到它可能只是代码风格的问题，但是我认为这样做更容易去遵守，并且让控制流更加的线性。

4、“Shy” code

就像The Pragmatic Programmer中提到的那样，你的代码应该永远是“shy code”。
尽量在最小范围内写每件事，只有在你真的需要的时候增加这个范围。
举个例子，从所有事情** **私有开始，包括你的属性和方法。

我认为这同样适用于属性的暴露级别，假如你有一个内部包含一个图片的自定义UI组件，同时它需要在外部设置，你可以提供一个更新它的方法，而不是将它作为一个可读写的属性。这样iamge就会作为一个只读的存在，并且你也可以在你属性的值上有更多的限制。

5、Remove empty methods and any unused generated code

The best code is the one that does not exist

要养成将不用的代码删除的习惯（哪怕是IDE自动生成的）。不要让你的代码中包含空方法，未用的变量，导入，无效的注释。如果你不需要它，那就删掉它。

删除代码应该是开发者最喜欢的活动之一。

一些其他的建议

当你review代码时，你应该牢牢的记住这些：

Hard-coded values：首先，你应该问自己，没有方法去替代这个静态值么？如果有，那么让它变得更好，如果没有，那就将它存储在静态变量中。这些值的命名应该和上下文有所联系，比如imageDefaultWidth 或者 controllerIdentifier，在有利于在一段时间后，容易理解这些值的意思。这样做的另一个好处是，这些值会在不同的地方重复出现。如果没有将它存在变量中，那么当你要做出改变的时候，比如将值从3修改为4，那么你将要在任何需要改变的地方做出修改。
建立团队标准和准则 - 确定标准，并且每个人都要遵守，不要在code review*时忽略这点。它应该包括变量的命名规则，代码规范，团队公认的开发语言好的尝试。
Log your errors - 或许这点是显而易见的，但是我还是认为，它值得提起。确保每次你的代码出现问题时都能及时记录，哪怕你认为它不可能发生，记下这个错误尽可能多的信息，它非常有用。

Objetive-c / swift specific observations

1、确保已经注销了观察者

每次review或者编写有关于注册某种通知或者订阅时，你都应该确认你是否注销或者取消订阅了。如果你忽略了这点，我相信你会知道“Message sent to deallocated instance”有多难debug。

我十分强调这一点，check，check，check。

2、Most specific method arguments

举个例子，button callbacks。应当为方法参数添加最详细的类型和调用对象。

好处体现在两个地方：一旦它更清晰，那么只会有一个button能调用这个方法，你不需要去检查对象的属性，不用描述它或者任何事情。你也许现在不需要，但是你需要为将来的开发流出空间。

@IBAction func didClickContinue(sender: id) { 
   // 一年后，估计你就记不起调用这个方法的是哪种类型的对象了
}

你应该用一些更加有意义的描述去快速的指出这段代码的实际用途

// 怎样使用这个方法？你会知道这个方法怎样用，也会知道哪个对象对调用这个方法
@IBAction func didClickContinueButton(sender: UIButton) {
}

关于方法：

// 永远都要将调用对象包含在callback中
let tapGesture = UITapGestureRecognizer(target: self, action: “didTap:”)

3、Most generic object references

当你使用一个对象时，尽可能的将它存在最通用的类型中，这不影响你使用你需要的功能

例如：

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

你需要注意的是，push方法只需要一个UIViewController而不是一个AlertViewController，所以你应该将它存在一个最通用的类型中比如UIViewController。

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

这里的想法是，不要在没有给出具体需求的时候，给出信息。* *对象之间联系越少，依赖就越小。

4、Documentation - Pargma marks

当一个类变得越来越大的时候（无论多么优秀的开发者，都会出现这样的问题），为public/private methods, the protocol implementations, public/private variables做一些限定，对于我们理解和工作都是非常有用的。

Other things to keep an eye for

循环引用 - 保证delegate是弱引用。同样在blocks中保证self 是weak，去避免循环引用。
不必要的代码 - 就像前面提到的，不要在你的代码中包含无用的代码。

（译）我的codereview习惯 的清单