我以审阅代码为生，以下是我最常见的审阅评论

你的代码读起来应该和你读过的任何其他引人入胜的故事一样。它应该从介绍你的主要“人物”开始，然后建立上下文，最后应该详细解释在什么时间发生在谁身上的事情。就像“契诃夫的枪”一样，如果你引入了某种价值，它一定是有目的的。

然而，仅仅在故事中有一个人的目的是不够的，为什么一个人是这样被介绍(或命名)的，这一点也应该不言而喻。

现在您一定已经听过无数次了，“代码只写一次，读几次”。因为这是真的。我甚至记不清我必须一遍又一遍地检查同一段代码多少次。有时，也许是为了找出一个bug，有时是为了向PM解释一些功能，有时是为了向新的团队成员解释一些功能。

因此，编写一段读起来像英文文本的代码非常有意义。这样以后就更容易理解了。并不是说人们想要故意编写神秘的代码。我的观点是，人们永远不会试图从别人的角度重新解读它。

当我们开始编写代码时，我们在头脑中构建了所有这些流程图、分支和逻辑，我们只是将它们作为一个过程放入编辑器中，并开始尝试使其工作。因为只有您编写了这段代码，所以理解它不会有任何问题。你记忆犹新，你知道为什么要设置这样的条件，但对将来阅读这段代码的人来说可能不是这样的。那个人甚至可能就是你自己。

我以审查代码为生，我看到了人们在编写产品代码时会遇到的一些常见问题。我不想谈论花哨的算法和用尽可能少的行重写代码的聪明方法。我相信你自己就能找到这些。

让代码变得愚蠢，让编码器变得聪明。愚蠢的代码总是比聪明的代码好。

以下是评论中重复次数最多的一些评论。这些评论大多是关于代码的可读性和可维护性的，其中一些是为了更好地调试。

字典是最懒惰的数据结构。当然，它又快又方便。但是不要过度使用它。如果您最终将字典作为函数参数进行传递，那么您就做错了事情。字典最多只能作为局部变量保存。

这有几个原因。首先，字典对于键/值映射没有任何约定，无论是基于键的存在还是基于类型。最终，您可能会对密钥进行大量的是否存在检查。这会使您的代码难看。在静态类型语言中，虽然您可以确保键上的类型，但您仍然必须检查键是否存在。

第二个原因是它的文档不好。代码的读取器不知道预期的键和值是什么。

例如，在上面的代码片段中，您不知道incomingEvent中需要什么样的键和值。您必须逐行阅读代码才能理解每个键/值对的用途。不过，对于您来说，很难清楚地描绘出传入事件的全部组成，因为它可能有一些当前在process方法中没有使用的键。

与此形成对比的是，拥有一个简单的对象就所有字段和值在传入事件中应该存在的方面而言显示了清晰的约定。这是更好的文档。

正确的数据模型能够在代码中尽可能完美地表示现实世界中的对象。

例如，我见过使用int表示数据库中一天中的时间的实例。类似于：

然后它进行了一次复杂的计算，从(Int)时间值中找出小时和分钟。

与其这样做，不如(虽然不完美)将它们保留为字符串，比如DND_START_TIME='；15：30'；。但是，如果可能的话，最好的想法是使用语言内置的对象来表示这一点。

类似的例子有作为整型或浮点型的货币、用于树/图的字典、用于布尔值的字符串(或整型)。

切勿更改您不拥有的任何数据。这意味着函数不应该改变它作为参数获得的值。这可能会导致灾难性的后果。例如，我曾经在一次公关中看到过这一点。

Def GET_ONLY_REQUIRED_VALIDATION(VALIDATIONS)：#放弃对`Executed`的验证，`filter_type`_=[validations.op(Key)for key in('；Executed'；，'；filter_type'；)]返回验证。

此示例从验证字典中永久删除EXECUTED和FILTER_TYPE值。如果呼叫者没有意识到这一点，这可能会在以后导致严重的问题。

Def GET_ONLY_REQUIRED_VALIDATION(验证)：如果k不在['；Executed'；，'；Filter_TYPE'；]中，则返回{k：v表示k，v表示validations.item()}。

这不会改变任何值，它会返回一个仅包含所需验证的新字典。

在构建API时，最好捕获所有异常并返回一个自定义异常，这样可以更好地传达异常的含义。

例如，以下几行是API的一部分，它从匹配的过滤器中返回第一个项目。

这种情况下的问题是从第一行查询返回的广告为空。在这种情况下，第二行将抛出IndexError。抛出像ItemNotFound这样的异常更好。

当函数参数在开始时没有检查None(NULL)值时，这种情况会发生很多次，当稍后在函数中使用时，它们会抛出异常，如`NoneType对象没有属性attr`(NPE)。最好在函数开始时检查NONE，并引发适当的异常或默认值为其他值。

但是，您不需要在自定义异常主体中不必要地包装相同的异常类型。如果现有异常在API上下文中仍然有意义，则可以跳过编写自定义异常类。例如，从缓存API抛出的KeyError和Value错误是有意义的。

一个函数在代码中不应该有太多的返回语句。它污染了控制流，使其更难遵循。尽量保留最多两个返回，1)第一级数据健全性检查2)最终结果。

Def COMPUTE_RESULT(a，b)：如果(a，b)中没有：返回0#某个默认值c=涉及a&；b的某个计算返回c。

在动态类型语言中，不检查返回值的数据类型。出于懒惰或任何其他原因，您可能会简单地在同一函数中的两个不同位置返回两种不同的数据类型。

这是一项严重的犯罪行为。不要那样做。这完全违背了所有的软件开发原则。它使整个软件的理智受到质疑；甚至可能使开发人员的理智受到质疑；)

从调试的角度来看，这是重要的问题之一。将日志放在所有意外的返回位置可以确保您可以更快地调试问题。

在上面的示例中，将一些日志放在返回之前。这将为您将来节省很多时间。

这是我注意到的另一件事，初级开发人员，有时也是高级开发人员，错过了。使用try-catch将循环体包围起来，以避免在其中一个循环体执行失败时意外中断循环。当然，除非这是你想要的那种行为。

不要使用不明确的函数或类名，因为它们要么没有任何特殊含义，要么没有表达类/函数应该做的事情。如果我命名两个函数类：ControlDatabaseQuery&；HandleDatabaseQuery，它们不会告诉代码的读者它们正在控制或处理的确切内容是什么。

在设计解决方案时，您可能最终会遇到当前系统状态与其应有状态不同的情况。可能是某些不能具有特定类型的值，或者是不能为空的数据容器，或者缓存中不应该出现在TTL之后的某些记录(如果它是这样设计的)，等等。

不要忽视系统的不一致状态。它们是系统中有问题的明显迹象。要么是开发人员在开发时的假设不正确，要么是某些数据已损坏。为此放置错误/信息日志，并且/或者如果您使用类似哨兵的服务，也应该在那里捕获它。

I)应该通过某个后台工作在到期时向服务注册的记录(或事件)。如果在另一个控制流中遇到a记录，而该记录在过期时间很久之后还没有注册。这是不一致状态的一个例子。

Ii)总是期望由某些终端用户输入并且没有默认值的某些字段值。有验证放入字段以检查是否为空。但是，如果字段值仍然为空，则这是另一种不一致状态。除非在数据模型中指定，否则不要在此级别采用任何默认值。您必须为此引发异常。