代码审查：最佳实践

最新推荐文章于 2022-09-09 22:55:40 发布

cunxiedian8614

最新推荐文章于 2022-09-09 22:55:40 发布

阅读量508

点赞数

文章标签：数据库 ui 数据结构与算法

不能低估代码审查在软件开发中的重要性。适当地进行代码审查不仅可以在早期阶段提高代码质量并识别潜在问题，还有助于培养开发人员的技能。但是，在大多数情况下，代码审查的意思是，只粗略地看一下所做的更改并提供一两个注释。没有多少人做详细和通过审查。如果您正在寻找如何更好地查看代码的方法，那么以下一些准则可能会对您有所帮助。在获得技术之前，让我们讨论一些非技术性的东西：

低估的第一点是，代码审阅可能会成为审阅者与开发人员之间的冲突点。开发人员（尤其是在职业生涯的成年时期）往往对他们编写的代码具有占有欲。他们的代码多么糟糕，他们认为这是有史以来最好的代码。在这种情况下，审阅者必须在评论中提供一些糖衣。在进行代码审查时，与开发人员相邻也比进行远程/断开代码审查更有帮助。

接下来，“代码审查”不会手动审查格式或任何静态分析。每个人都会对制表符/空格或同一行/下一行中的括号等项目有自己的偏好。最好以某种方式解决它（高级人员的独裁多数有效，但是，是的，打架是一种解决方案也是如此），其他所有人都可以遵循。最好将其在“代码格式”首选项中集成到您喜欢的IDE中，以便在您键入/保存时对其进行格式化。同样，您可以通过各种工具执行所有静态代码分析（例如潜在的空指针异常或不建议使用的方法），并将其集成到IDE和构建过程中。

©XKCD

最后，不要提供主观评论。它只会导致碰撞。当您提供任何评论注释时，如果评论/请求的更改不明显，则需要提供扎实的更改理由。这有助于在Reviewer和开发人员之间建立长期的关系，也有助于开发人员了解他不了解的事物。

现在，让我们看看代码审查的目标。

确保更改确实满足功能要求如果可能，找出任何潜在的问题（一次性错误，潜在的性能问题等）具有可读性和可维护性

在代码审查中应遵循并验证以下准则：

Naming:

“其他名字的玫瑰都会闻起来很甜”。好吧，让它交给莎士比亚，是的。相同的逻辑不能应用于编程中的类/方法/变量的命名。假设从数据库中删除实体的方法最好命名为Entity.delete（）。诸如Entity.hide（）或Entity.remove（）之类的任何其他名称与delete（）都不相同和/或有效。

一个现实的例子。多年前，我遇到了一个名为Tree.seachForNode（name）的方法。它被实现为遍历树中的节点，如果找到名称相同的节点，则返回相同的节点如果找不到，则使用该名称创建一个节点；更新树以具有该节点；将节点插入数据库并返回节点

想象一下该方法的调用者的困惑。没有人会期望数据库插入将是“搜索”方法的结果。如果该方法名为createIfNotExists（），则不是这种情况。

因此，意图公开名称对于理解和维护代码库非常重要。

Falls in line with the architecture:

软件体系结构与设计选择有关。使用什么框架；各个模块是什么？他们如何与自己互动；使用哪些库？什么是跨领域关注的问题以及如何实施。

Single Responsibility:

每个方法和每个类都只能做一件事，并且做得很好。在审核过程中，我们应该验证这确实发生了。在实际情况下，不可能只承担一种责任，我们应确保该方法至少被分解为较小的方法，每种方法都有一个责任。在上面的示例中，createIfNotExists（）具有两个职责：（1）搜索节点，（2）创建节点。最好将功能分为两个不同的方法Tree.search（）和Tree.create（），而Tree.createIfNotExists（）只是调用这两个方法的包装。

Abstraction:

每个层/类/方法应具有适当的抽象。

在分层体系结构中，每一层都应有效地隐藏基础层的复杂性，并为上面的层提供抽象。例如，数据访问层将有效地处理数据存储的复杂性（外键，事务等），并提供可用于工作的逻辑模型。服务层将采用逻辑模型并公开功能。 UI / API层将使用这些服务并提供UI或API。现在，每个层都有一个抽象。业务层不应在外键上运行或处理HTTP请求/响应。

同样，如果一个名为BinaryTree的类试图解析配置文件以读取某些配置，那也是泄漏抽象。解析和设置配置和环境变量最好由另一个类完成，而BinaryTree的抽象应仅处理实现二叉树的操作。

Provide correct level of Abstraction:

在面向对象设计中，抽象为您提供了一种更轻松的交互方式，隐藏了其所有内部。不要过度暴露客户应该了解的内容。例子：

该方法/变量真的应该公开吗？还是可能会受到保护？在某些情况下，属性只能在类中更改，而客户端应具有对该属性的只读访问权限。在那种情况下，需要有一个公共的getter方法，但是为什么要有一个公共的setter方法呢？getChildren（）方法是否返回List？如果客户修改列表怎么办？ Tree类会自动适应吗？可能是我们应该返回一个Enumerable，以便客户端可以迭代和访问子级，但是不能修改集合？

提供正确的抽象级别，这样就不会有任何意外的行为。

Method contract:

每种方法都有合同。合同包括前提条件，执行/行为和前提条件。例如，让我们考虑insert（TreeNode）方法，该方法将新记录插入数据库。可能的前提条件是（1）TreeNode不应为null，并且（2）name字段不应为null。执行/行为会将行插入到数据库中，而后条件是数据库中的记录应该存在并具有给定的值。

前提条件必须在执行开始之前进行验证。另外，还假定满足前提条件的责任在于此方法的调用方。前提条件中的任何失败，该方法都将导致异常。如果执行一切正常，则该方法应确保满足后置条件。如果出于任何原因无法满足后期条件，则应再次导致Exception。

insert方法的典型实现可能是：

void insert(TreeNode node){  

//Pre-Condition 1  
if(node == null)  
   throw new IllegalArgumentException("Parameter 'node' cannot be null");  

//Pre-Condition 2  
if(node.getName() == null)  
   throw new IllegalArgumentException("'Name' of the node cannot be null");  


//Execution  
try(get the Database Connection){  
   // insert into record into the database  
}catch(DuplicatePrimaryKeyException e){  
   throw new InvalidDataException("A node with the name '"+node.getName()+"' already exists");  
}  
}

如果您已经注意到，在我们验证前提条件的同时，此处不会进行条件前提条件的验证（无论记录是否存在给定值的数据库中）。通常在此方法的单元测试中对此进行验证。

Choosing the right Data Type:

为减少错误，为给定数据选择正确的类型至关重要。很多时候，我遇到过用字符串代替枚举的情况。如果值列表是有限的并且在编译时已知，那么为什么要使用字符串？例如：性别。

List不仅是枚举，而且是另一种广泛使用和错误使用的类型。考虑一下：

List excludedExtensions = ReadFromConfig();

列表用于存储值的有序集合。值也可以重复。在这种情况下，要排除的文件扩展名将是唯一的，并且顺序并不重要。他们为什么要使用List？与列表相比，此处Set是更适合的数据类型。

Quick note on working with legacy codebases:

如果您使用的是旧版代码库，则很可能它不是您所见过的最干净的代码（如果您希望它不加糖衣，那就是一团糟）。清除它的唯一方法是一点一点地完成它。您所做的每次提交都应使代码库比以前更干净。如果无法做到这一点，至少要确保它不会变得更脏。慢慢修复它需要大量的耐心和时间。

©未知

from: https://dev.to//grprakash/code-review-guidelines-3fad