我的代码检查清单

（由我们自己的Mobster Sameh Mabrouk ，高级iOS开发人员 @Mobiquity贡献 ）

代码审查的定义：

根据维基百科：

代码审查是计算机源代码的系统检查（有时称为同行审查）。 它旨在发现在初始开发阶段被忽略的错误，从而提高软件的整体质量。

好吧，我想这样做！ 等等，但是……

怎么做？

可以采用多种形式进行审核，例如配对编程，非正式演练和正式检查。 最有名的大概就是这个-给我看看您的代码（又名非正式评论）！ 只需让您的同伴查看您的代码即可。

可以使用不同的工具（例如Atlassian的Crucible / Fisheye或BitBucket / GitHub上的拉取请求）或任何您使用的工具来执行正式代码审查。 借助这些工具，您可以很好地看到对源代码所做的更改，可以对它们进行注释，向作者提出一些问题，然后他们可以解释其代码。 就像您在现实生活中进行的对话一样，但是要记录在案–同意的内容应该在合并成开发之前执行。 等待，什么合并？ 我们只是在谈论评论…

Git流

在工作中开发应用程序的不同部分时，我们使用Git和Git Flow将所有更改合并到父分支中。 应用功能完成后，我们将创建一个请求请求，其中包含要添加到先前分支（通常是开发 ）中的更改。 我认为，显示流程的最佳图片来自GitHub。

流程如下所示：

  • 从开发创建分支
  • 将您的更改应用于源代码
  • 创建一个合并到例如开发中的拉取请求
  • 与同事讨论变更，解释您的观点并应用建议的改进措施
  • 您的同伴批准您的更改
  • 将您的代码合并到源分支

进行审核时应该注意什么？

您绝对应该检查代码的完整性-样式是否与以前的解决方案匹配，是否遵循约定的约定？ 功能是否正确实现（关于这一点，我会建议PR创建者是否可以添加.GIF图像来说明功能的工作原理，这样很好） ，更改后旧的源代码是否可以正常工作？

为什么要在开发过程中关心代码审查？

可以肯定的是，由于它确保代码完整性（），因此可以捕捉到其他人看不到的东西。 它可以让您学习和分享您的知识和专长，通过与您共同的话题（代码和编程技能；）的对话，加强团队中的沟通并建立良好的关系；）！

就我个人而言，最近在我当前的项目中已经看到了。 团队在沟通方面存在问题，但是在我们进行了代码审查的对话之后，沟通变得更好了。

将代码审查视为对未来的投资。 如果您现在不捕获错误，那么将来一定要征服它们。

如果您不执行代码审查该怎么办

想象一家X公司，它为多个客户提供移动应用解决方案。 他们的团队很小，有时甚至无法满足客户的需求。 因此，他们决定将一些工作外包给外部公司。 他们向该公司提出项目要求，并在3个月后再次开会以接收应用程序源代码。 但是该应用程序无用，它在网络调用时冻结， CoreData.ConcurrencyDebug标志CoreData.ConcurrencyDebug崩溃。 该项目被延迟了几个月，团队不得不从头开始。 他们希望他们每天都审查外包的代码…

您要执行代码审查吗？

这通常是观众缺乏热情的问题：(。但是，实际上，我们是否忙于改进？

这是代码审查的简介。 现在让我们更深入地进行代码审查过程，在执行审查时提供一些提示和技巧，并在审查快速代码时着重于一些错误。

作为iOS开发人员，到目前为止，我想到的大多数东西都与iOS平台相关。Swift是我的主要编程语言，但是我认为有些通用的东西与平台或特定的编程语言无关。 所以让我们把它们说出来

一般提示：

据说，一次审查少于400行代码时，审查效果最好。 您应该进行适当且缓慢的审核，但是，不要花超过90分钟的时间⏰

那之后你肯定会累的。 编写和理解您自己的代码很累，而理解某人的代码则更累。

经过几年的软件开发经验，您知道什么是常见的编程错误。 您还将意识到对常见问题的解决方案最有效。 写下来并对照检查清单code检查正在检查的代码是一个好习惯-这样可以改善结果。 清单对审阅者尤为重要，因为如果作者忘记了一项任务，审阅者也可能会错过它。

在上面提到的git流程中，代码可以在收到同意的拉取请求批准数量后合并。 该数字应取决于您的团队规模，但最好至少获得⓵！😀

当您建议对代码进行修复时，请提出其重要性💬。 也许其中一些可以推迟并提取为单独的任务。 在批准拉取请求之前，请确保已修复缺陷🔧🔨。

在您的公司中传播良好的代码审查文化，在这种文化中，对缺陷的发现会得到积极的评价 viewed 软件代码审查的重点是消除尽可能多的缺陷 ，而不管是谁“引起”了该错误。 没有什么比得到同行的称赞更好。 奖励开发人员的成长和努力。 提供尽可能多的正面评论。 我总是试图说一个解决方案既好又聪明（如果确实是😉）。

您也可以从“自我效应”中受益。 自我效应促使开发人员编写更好的代码，因为他们知道其他人将关注他们的代码和指标。 没有人愿意被称为犯所有这些初级错误的人 。 自我效应促使开发人员在将自己的工作传递给他人之前仔细检查自己的工作 。

并且不要理会代码格式化样式……

…与争论☔️关于空白位置相比，有很多更好的事情可以花时间。

寻找错误和性能问题是主要目标，也是您应花费大部分时间的地方。

我在审查中寻找什么？

1.风格

常见的红旗🚩

1.干（不要重复自己）

对于DRY原理，有很多示例，让我们以最简单的一个为例：

DRY的简单含义是不要重复编写相同的代码。 让我们以以下代码为例： 

  //假设我们有一个布尔值，并且以false初始化 
 让isPositionCorrect = false 
 如果isPositionCorrect { 
  updateUI（“位置正确”，isPositionCorrect） 
  }其他{ 
  updateUI（“ Position InCorrect”，isPositionCorrect） 
  }

如果我们看一下这段代码，就会发现对updateUI函数的调用被重复了两次。 现在这是一个不好的代码，因为从可维护性的角度来看，如果方法定义已更改，那么您需要对其进行两次更改，例如，现在该方法需要2个参数，而如果我们需要将其更改为3个参数，则必须在2个位置进行更改。

要解决此问题，我们必须遵循DRY原则： 

 让message = isPositionCorrect吗？  “位置正确”：“位置不正确” 
  updateUI（message，isPositionCorrect）

因此，在这种情况下，如果我们需要更改功能，我们将仅更改一个位置，不再重复。

2.使用提前退出

循环越多，我们就需要更多的精力来理解它。 因此，通过提早退出而不是将代码包装在一个if循环语句中，您可以添加不首先执行循环的条件，如果是这种情况，则只需返回即可 。

示例，而不是： 

 函数功能（项：[Int]）{ 
 如果items.count> 0 { 
 用于项目{ 
  // 做点什么 
  } 
  } 
  }

您可以这样写： 

 函数功能（项：[Int]）{ 
 如果items.isEmpty { 
 返回 
  } 
 用于项目{ 
  // 做点什么 
  } 
  }

更快速的风格，您可以这样写： 

 函数功能（项：[Int]）{ 
 守护！items.isEmpty else {return} 
 用于项目{ 
  // 做点什么 
  } 
  }

3.返回布尔值： 

  func isItemExist（x：Int，items：[Int]）->布尔{ 
 如果items.contains（x）{ 
 返回真 
  } 
 其他{ 
 返回假 
  } 
  }

如果您的函数返回布尔值，请仔细查看一下，看看是否可以用一种痛苦的方式编写它，因为在大多数情况下，可以用一种更简单的方式编写它。 

  func isItemExist（x：Int，items：[Int]）->布尔{ 
 返回items.contains（x） 
  }

另外，如果要处理对象的布尔值： 

  func showImageViewIfEmptyItems（imageView：UIImageView，items：[Int]）{ 
 如果items.isEmpty { 
  imageView.isHidden = true 
  } 
 其他{ 
  imageView.isHidden = false 
  } 
  }

可以这样写： 

  func showImageViewIfEmptyItems（imageView：UIImageView，items：[Int]）{ 
  imageView.isHidden = items.isEmpty 
  }

4.删除未使用的代码

最好的代码根本没有代码

删除代码应该是您作为开发人员的活动之一。 始终删除未使用的代码以及IDE生成的代码。 在代码审查中，当您看到空方法，未使用的变量，过时的注释，代码周围挂着导入内容时，不仅要保留它，还应将其删除。

注释掉的代码应删除。 不要把它挂在那儿。 实际上，您在源代码管理工具中拥有所有历史记录。 根据我的经验，我认为您很少回到最初将其注释的代码。 如果您想返回此注释代码，则只需还原文件即可。

有时我在我审查的代码库中找到类似的东西： 

  func foo（String：item）-> String { 
 让baz = findKeyForItem（item） 
  //由于某些原因，我们不再这样做 
  // if（baz ==“ foobar”）{ 
  //返回baz 
  //}其他{ 
  //返回item.foobar（） 
  //} 
 返回巴兹 
  }

该函数应如下所示： 

  func foo（String：item）-> String { 
 返回findKeyForItem（item） 
  }

现在，如果您需要返回旧代码，只需使用git diff（如果您更喜欢命令行），则可以使用与IDE或首选的源代码控制工具集成的git客户端。我们可以收获这些变化和繁荣！ 我们回来了。

5.编写“害羞”代码

如The Pragmatic Programmer中所述 ，您的代码应始终为“害羞代码” 。 编写不会与太多事物交互的害羞代码。 编写害羞的代码，使对象松散耦合。 用可能的最小作用域编写所有内容，并仅在确实需要时才扩大作用域-例如，从一切私有 ，属性以及方法开始。

此规则适用于对象拥有的属性的说明级别。 假设您有一个自定义的UI组件，其中包含一个图像，该图像也应从组件外部进行设置。 您可以提供一种方法来更新类中的相应属性，而不是将图像公开为可读写属性（在Swift中，我们会使用计算得到的属性来执行此操作，因为我们可以获得并设置此属性） 。 该图像将仅显示为只读，并且您还可以对在属性上设置的值进行更多控制。

例如： 

 类ReusableUIComponent { 

 私人var图片：UIImage 

  init（image：UIImage）{ 
  self.image =图片 
  } 

  var componentImage：UIImage { 
 得到{ 
 返回图片 
  } 

 设置{ 
 图片= newValue 
  } 
  } 
  }

6.硬编码值

始终考虑使事物可配置，而不是硬编码值。

这是审查代码的琐碎活动之一。 每当您看到硬编码的值时，请先问自己是否还有另一种方法可以删除此静态值（字符串，浮点数，整数等）并使之可配置？ 然后去改善它。 如果没有办法使用它，请将其存储在静态变量中（通常这将在整个项目中使用的常量类/文件中）。

这样做的好处显然是避免代码重复。 无需将此静态值存储在变量中，而是需要在值更改时在所有位置更改代码。

7.语言风格指南和编码约定

几乎每个公司都应拥有自己的语言样式指南和代码约定，例如Github的快速样式指南。 始终确保每个人都遵循编码风格和团队标准。 因为以您的方式做事可能会更有趣，但是您的同事可能不习惯您的编码风格，并且如果不寻常，那么下一位开发人员将很难处理您所构建的内容。 您的审查应包括一致的变量命名方式，代码格式，团队中商定的语言最佳实践。

2.建筑/设计

  • 该代码应遵循已定义的体系结构，无论是MVC，MVP，MVVM还是VIPER体系结构，请确保您的同行都遵循它。 还请注意可能对您的项目有用的新模式。
  • 检查提交的代码是否可重用。 考虑泛型函数和类。
  • 检查提交的代码是否正确设计并遵循团队使用的软件模式，因此，如果您的团队遵循TDD（测试驱动开发） ，则应始终在生产代码之前编写测试，这意味着您的代码设计完美。 根据我的经验， TDD节省了很多时间来设计代码，但是如果TDD在设计代码时没有增加价值，那么就不要这样做，直接编写生产代码。
  • 检查提交的代码是否为干净代码。 它应遵循面向对象的分析和设计（OOAD）原则或SOLID 原则。

SOLID原则：

  1. S —单一职责原则（SRS）：不要在单个类或函数中放置多个职责，而在不同的类和函数（纯对象）中进行重构。
  2. O —开放封闭原则：在添加新功能时，不应修改现有代码。 新功能应该用新的类和功能（扩展名）编写。
  3. L-Liskov可替换性原理： 子类不应更改父类的行为（含义）。 子类可以替代基类。
  4. I —接口隔离： 不要创建冗长的接口，而是根据功能将它们拆分为较小的接口。 该接口不应包含任何依赖关系（参数），这对于预期功能不是必需的。
  5. D —依赖倒置原则：高级模块不应依赖于低级模块，而应依赖于抽象（协议）， 这使得依赖注入变得容易。

3.测试

  • 单元测试：检查是否已针对新功能编写了单元测试。 它们是否涵盖了故障条件？ 它们容易阅读吗？ 他们有多脆弱？ 测试有多大？ 他们慢吗？

一些Swift的相关代码审查

1.砰砰或用力解开❗

在查看Swift代码时，我要寻找的一件事就是这个感叹号－总是在寻找使用它的代码。 

  var foo：对象！ 
  print（“ \（foo.someString）”）

如您所见，此print语句将导致崩溃，因为我们访问未初始化的foo变量。 总是问自己，没有更好的方法吗？ 也许是一个守卫，可选的链接。

确保您的同伴（以及您自己）在其代码中明智地使用爆炸/强制展开操作符。

2. autoreleasepool

您还记得autoreleasepool吗？ 如果你不知道那是什么 这里 是一些解释。 在查看循环主体以检查是否可以使用本地autoreleasepool减少峰值内存占用量时，我总是很注意。

例如，在这段代码中，我们创建了5次UIImage 

 func useManyImages() { 
 let filename = pathForResourceInBundle 
  for _ in 0 ..< 5 { 
 let image =   for _ in 0 ..< 5 { 
 let image = UIImage (contentsOfFile: filename) 
 } 
 } (contentsOfFile: filename) 
 } 
 }

为了减少峰值内存占用，我们应该像这样使用autoreleasepool ： 

 func useManyImages() { 
 let filename = pathForResourceInBundle 
  for _ in 0 ..< 5 { 
 autoreleasepool { 
 let image =   for _ in 0 ..< 5 { 
 autoreleasepool { 
 let image = UIImage (contentsOfFile: filename) 
 } 
 } 
 } (contentsOfFile: filename) 
 } 
 } 
 }

3.可变与不变属性

如果对象的属性取决于某些值，则永远不要使用var。 使用let。 总是比var更重视不变的状态。 如果您的资产确实必须是可变的，并且另一个对象需要像单元测试那样访问它，则将其作为不可变对象公开给公众，如下所示： 

  private（set）var foo：AnyObject 
 让吧：AnyObject

4.删除/注销观察者

每当您查看或编写注册某种通知或作为观察者订阅更新的代码时，请确保您检查它们是否也注销/取消订阅。 请检查一下，因为我们都知道调试“发送到已释放实例的消息”的难度。

5.保留周​​期

使用委托时避免保留周期，请确保委托是弱者。 此外，在使用积木时，请确保将自己捕捉为弱者。

一个例子： 

  Foo类{ 
 让bar = Bar（） 
  bar.closeAsynchronous {[unown self] 
  self.barIsClosed（） 
  } 
  func barIsClosed（）{ 
  println（“ Bar”） 
  } 
  }

上面的代码中包含一个保留周期。 在不阻塞主队列的情况下，会在一段时间后调用结尾的闭包closeAsynchronous {…} 。 需要[无名的自我]来制动周期。 如果bar不是Foo的变量，则我们不需要[unown self]

6.扩展执行的代表

通过确保委托在扩展中实现，确保代码可读性和可维护性。

一个例子： 

  Foo类{ 
  } 
  Foo扩展名：委托{ 
  func RepresentativeMethod（）{ 
  } 
  }

7.使用大多数通用对象引用

始终确保在使用对象时，将其保存到具有最通用类型的变量中。

请参阅以下示例： 

 私人var viewController：UIAlertController？ 

 在里面（） { 
  viewController = UIAlertController（） 
  navigationController？.pushViewController（viewController，animation：true） 
  }

在这里，如果您注意到pushViewController方法仅需要ViewController而不是UIAlertController对象。 因此，由于我们不需要UIAlertController的任何属性， 因此可以将其保存到ViewController中，如下所示： 

 私人var viewController：UIViewController？ 

 在里面（） { 
  viewController = UIAlertController（） 
  navigationController？.pushViewController（viewController，animation：true） 
  }

这里的想法是不提供不需要的更多信息。 对象之间相互了解的越少，代码中的依赖性就越少。

8.单元测试

在单元测试中使用！ 强制展开，因为如果在展开时崩溃，这也意味着测试失败。 作为示例，请参见testForcedUnwrap 

 类Foo（）{ 
  func getOptional（）->类型？  { 
 返回类型？ 
  } 
  } 
  func testForcedUnwrap（）{ 
 让foo = Foo（） 
 让optional = foo.getOptional（）！  //此处发生崩溃表示测试失败 

  XCTAssertNotNil（可选） 
  }

9.代码文档

当类太大时，为公共/私有方法，协议实现，公共/私有变量以及任何可帮助任何人理解和使用代码并进行导航的内容添加某种分隔符（实用标记）很有用。上课比较容易。

最后的想法：

代码审查是一种心态，而不是过程。 在代码审查期间，请保持开放的态度。 我认为这是每个人都在努力的事情。 当有人说您编写的代码可能会更好时，请尽量不要采取防御措施，也不要将其用于个人。

如果审阅者提出了建议，而对于为什么不应该执行该建议我没有明确的答案，我通常会进行更改。 如果审阅者在询问有关一行代码的问题，则可能意味着将来会混淆其他代码。 此外，进行更改可以帮助发现较大的体系结构问题或错误。

对我来说重要的方面之一是，没有什么可以评论的。 根据破窗理论 。 这也适用于软件开发。

始终如此 -请先检查您自己的代码！ 在提交代码之前，请仔细检查您所做的所有更改 （差异） ！

另外，您应该使用Lint工具，例如SwiftLint ，这将使您轻松设置自动代码查看准则。 只需您可以自动执行此清单中的某些项目，并在进行代码审查时节省时间。

这些是我在过去几年中与非常有才华和热情的人一起工作时进行代码审查时考虑的基本内容。

我将始终保持清单打开，并在出现新内容时对其进行更新。

Interesting Posts