如何在代码审查中找到积极的东西？

#1 楼

如何在代码审查中找到积极的东西？
在去年发生了一些严重的质量问题之后，我公司最近推出了代码审查。

太好了，有机会为您的公司创造价值。

最初的几周后，我的同事开始放任自流，以免对同事造成麻烦（她告诉我自己，
客户提交了一个错误报告，她知道该错误，但
担心开发人员会因为指出错误而对她感到生气）。

您的同事应该如果她不能告诉开发人员代码有什么问题，请不要进行代码审查。找到问题并在问题影响到客户之前解决问题是您的工作。
同样，恐吓同事的开发人员也要被解雇。经过代码审查后，我感到很害怕-我告诉我的老板，它已得到处理。另外，我喜欢我的工作，因此我保持正面和负面的反馈。作为审阅者，这是我的责任，而不是其他任何人。 >好吧，很不幸，您说自己很机智。如果您有更多需要的地方，可以找到更多值得称赞的东西。
批判代码，而不是作者
您举一个例子：在

中避免使用单词“ you”和“ your”，而改为“ the”。

我错过了什么吗？ [...]为什么？

不要在您的评论中添加夸张的夸张。也不要开玩笑。我听过一个规则，“如果让你觉得好说，不要说，那就不好了。”
也许你是在自欺欺人，以别人为代价。坚持事实。
通过提供积极的反馈来提高标准
提高标准，以表彰您的开发人员达到更高的标准。这就是问题所在，

如何在代码审查中找到积极的东西？

是一个很好的例子，值得解决。
您可以指出代码在哪里符合更高级别的编码实践的理想。
寻找他们遵循最佳实践，并不断提高标准。在每个人都期望获得更简单的理想之后，您将不应该再赞扬这些理想，而是寻求更好的编码实践来称赞。面向功能的或功能性的编程功能，您可以大声疾呼，并祝贺作者在适当的地方使用它们。这些问题通常属于样式指南：

是否符合内部语言样式指南标准？
它是否符合该语言最权威的样式指南（可能比该指南更严格内部-因此仍然符合内部风格）？

通用最佳实践
您可以在各种范式下找到赞美通用编码原则的要点。例如，他们是否具有良好的单元测试？单元测试是否涵盖了大多数代码？代码覆盖率，以及对API和语义公共功能的完整测试。
验收测试和冒烟测试可以测试端到端功能，包括为单元测试模拟的功能。要点，所以代码是DRY（不要重复自己），没有魔术字符串或数字。
变量的命名是如此出色，以至于注释在很大程度上是多余的。
清理，客观改进（不需权衡）和适当的重构。可以减少代码行数和技术负担，而又不会使代码与原始作者完全陌生。

函数式编程
如果语言是功能性语言，或支持功能性范式，请寻找以下理想：

使用闭包和部分函数避免全局和全局状态
具有可读性，正确性，和描述性名称
单个出口点，最大程度地减少了参数数

面向对象编程（OOP）
如果语言支持OOP，则可以赞扬这些功能的适当用法：

封装-提供一个清晰定义的小型公共接口，并隐藏细节。
继承-可能通过mixins适当地重用了代码。
多态性-定义了接口，也许是抽象的基础类，为支持参数多态性而编写的函数。

在OOP下，还有SOLID原则（可能对OOP功能有一些冗余）： / owner
打开/关闭-不修改已建立对象的接口
Liskov替代ion-子类可以代替父级实例
接口隔离-由组合提供的接口，也许是mixins
依赖性反转-定义的接口-多态...

Unix编程原理：
Unix的原则是模块化，清晰度，组成，分离，简单，简约，透明，健壮，表示，最小惊喜，沉默，修复，经济，生成，优化，多样性和可扩展性。
这些原则可以在许多范式中应用。
您的条件
这些都太琐碎了-如果对此赞誉有加，我会觉得很屈从：

代码将编译。 />至少有一种方法可以使用该代码。
该代码可以在大多数普通情况下使用。

另一方面，考虑到您的实际情况，这些都是相当受好评的正与之打交道，我会毫不犹豫地称赞开发人员所做的事情：

该代码将适用于大多数情况。
如果插入的数据无效，则代码将引发合理的异常。

写下通过代码审查的规则？
从理论上讲，尽管我通常不会因为错误的命名而拒绝代码，但我看到命名是如此糟糕，以至于我会拒绝带有修复指令的代码。您必须能够出于任何原因拒绝代码。
我能想到的唯一拒绝代码的规则是，没有什么太可怕了，我无法将其拒之门外。我很乐意将一个不好的名字拒之门外-但您不能将其作为规则。如果语言支持它们，那么所有这些。

#2 楼

除非有一个可靠的简洁示例，并且与所关注的问题直接相关，否则请不要去挑选一些好的东西。对自己的能力不安全并且正在以不成熟的方式应对工作挑战的人。他们的工作也可能很糟糕-优秀的开发人员应该始终愿意反省，接受建设性的批评，并乐于改变自己的方式。您。不管您是否认为自己很合理，都需要对这样的人格外小心，以使事情顺利进行。我发现与这些人打交道的最好方法是，对自己的用语表达要非常小心。

确保您在责怪代码而不是作者。专注于手头的一个问题，而不是您的代码库的poo-mountain，他们可能在创建过程中发挥了重要作用，并被视为进一步的人身攻击。首先选择战斗，将注意力集中在向用户显示的关键问题上，这样就不会对导致他们拒绝您所说的一切的人发起批评。如果您要亲自与他们交谈，这很重要，请清楚您在说什么，并确保您没有与他们交谈或忽视他们的技术能力。他们很可能会立即采取防御措施，因此您需要解决他们的担忧而不是确认他们。您需要对这次谈话保持意识，不要太明显，以使他们下意识地认为您站在他们一边，并希望他们接受他们需要做出引起注意的更改。如果这不起作用，您可以开始变得更具攻击性。如果该产品可在会议室中运行，则在代码检查期间将其放在投影机上，并第一手显示该错误，如果经理在现场，则该功能更好，这样人们就不能退缩。这不是要让他们感到羞耻，而是要迫使他们接受这个问题，对用户来说是真实的，并且需要解决它，而不是仅仅困扰于他们的代码。您无所事事，厌倦了像幼儿园学生那样对待人，并且管理层完全没有意识到问题，这或者反映了您作为代码审查员的表现不佳，或者您关心公司的健康状况， /或产品，您需要开始与老板讨论他们的行为。尽可能具体和无个性-提出一个业务案例，证明生产力和质量正在受到损害。

#3 楼

代码审查可能是有毒的，浪费时间，会激怒活泼的书呆子战争。只需看一下对干净代码与注释，命名约定，单元和集成测试，签入策略，RESTfulness等等问题的意见分歧。写下通过代码审查的规则。

然后，不是失败或批准签到的人。他们只是在检查是否已遵守规则。

由于已预先记录下来，因此在编写代码时，您可以遵循规则，而不必在后面解释原因或争论。

如果您不喜欢这些规则，请开会修改。

#4 楼

我不会加糖，因为它可能被认为是光顾的。

我认为，最佳实践是始终专注于代码而不是作者。

这是代码审查，而不是开发人员审查，因此：


“此while循环可能永远不会结束”，而不是“您的循环可能永远不会结束” br />“方案X需要一个测试用例”，而不是“您没有编写测试来涵盖该方案”。

在讨论有关方案时应用相同的规则也很重要与其他人一起审查：


“安妮，您对这段代码有什么看法？”，而不是“安妮，您对乔恩的代码有何看法？”代码审查不是进行性能审查的时间-应该单独进行。

#5 楼

令我惊讶的是，到目前为止，在任何答案中都没有提到它，也许我在代码审查方面的经验是不寻常的，但是：
为什么要在一条消息中审查整个合并请求？
我在代码审查方面的经验是通过GitLab进行的；我一直以为其他代码审查工具也可以类似地工作。这极不可能被个人批评，因为它是对代码的注释，并且实际上是作为对代码的注释显示的，直接在要注释的代码下方显示。
我也可以对代码进行注释整个合并请求，并且经常这样做。但是可以在差异的特定行上指出特定点。 （此外，当添加新的提交以使diff发生变化时，默认情况下将隐藏对“过时的diff”的注释。）
通过此工作流程，代码审查变得更加模块化和内聚。来自代码审查的一行可以简单地说，

好方法，将其包装在专用函数中！对象名称与对象的用途并不完全匹配；也许我们可以改用“ XYZ”之类的名称？

或者如果某节存在重大问题，我可以这样写：我知道它为什么起作用），但是需要进行专门的研究才能理解它。您能否将其重构为单独的函数，以便将来进行维护？
（示例：函数ABC实际上在这里做三件事：弄糟foo，禁止boz和弄皱zorf。是单独的函数。）

完成上述所有操作后，我可以对整个合并请求进行摘要注释，例如：这是一个很棒的新功能，一旦合并，它将非常有用。您能否清理函数命名，并处理我所做的个别注释中提到的重构，然后让我知道再次查看它？ :)


即使合并请求是完整的早餐，单独的注释也可以很简单。他们将更多。然后，摘要注释可能会说：

很抱歉，但是这段代码并没有真正使人窒息。在很多情况下（将在个别注释中详细介绍），这些情况会被错误地处理，并给用户带来不良的体验，甚至在一种情况下还会破坏数据。 （请参阅有关提交438a95fb734的注释。）即使是某些正常使用情况，也会导致极差的应用程序性能（具体说明参见somefile.c的diff注释）。更加注意在流程的每个点上可以安全地做出哪些假设。 （提示：除非经过检查，否则所有假设都是不安全的。）
我正在关闭合并请求，等待完全重写。


摘要：检查技术方面的代码作为对各行代码的注释。然后在合并请求的整体注释中总结这些注释。不要变得个人化，只需处理事实，并在您看来是关于代码的，而不是关于编码者的。并基于事实，准确的观察和理解来发表您的看法。

#6 楼

我真的很惊讶，没人能接受，但是发布的示例评论有问题。

没有理由直接与Joe联系。乔修复自己的失败与您无关。有人做是您的事。您关心的是代码质量。因此，不要写请求/命令/需求（如果我是Joe，我可能会因为您不合法而拒绝了），而是继续扮演自己的角色，并编写一个简单的匿名待办事项清单。

要公正地给出好与坏点不仅是不可能的，而且完全超出了您的角色。

以下是重新编写示例的内容，内容摘自您的评论：


在对Library \ ACME \ ExtractOrderMail类进行更改之前，我们需要修复一些问题。
除非我错过了一些内容，否则“ TempFilesToDelete”不应是静态的。我们可能每次运行都多次调用该函数，这就是为什么我们需要这样做（必须在此处完成）。 （而且，我现在处于临界点，因为到现在为止，您应该已经有了结论）
应该重命名SaveAndSend以更好地适应其行为，例如“ SendErrorMail”
注释掉的代码应该出于可读性目的而删除。我们使用Subversion进行最终的回滚。 。 WHO ？什么时候 ？没人在乎。什么 ？为什么呢应该说。

现在，您将通过消除人为因素来解决代码审查日益紧张的原因。

#7 楼

代码审查的重点是发现问题。如果有任何错误，最好的办法是编写一个失败的测试用例。

您的团队（经理）应该传达出产生错误是游戏的一部分，但是要找到并修复它们将挽救每个人的工作。

召开定期会议（团队会议或双人会议）并解决几个问题可能会有所帮助。最初的程序员没有故意引入问题，有时他可能会认为它足够好（有时甚至可能）。但是，第二只眼睛给人的感觉是全新的，他可能会从解决这些问题中学到很多东西。

这确实是一种文化，需要很多信任和沟通。和时间来处理结果。

#8 楼

我认为积极的事情是在进行审查之前就编码标准达成共识。其他人在输入信息后往往会更多地购买某些东西。大多数开发人员会在同行之间达成共识。谁愿意成为不想同意编程世界中如此普遍的事物的人？当您通过选择某人的代码并指出漏洞来开始该过程时，他们会变得非常防御。建立标准后，在解释或“足够好”上会有分歧，但您的状况比现在要好。

此过程的另一部分是确定代码审查的方式将处理异议。这不能成为无休止的辩论。在某些时候，必须由某人做出决定。也许可以有第三方担任法官，或者审阅人获得所有权力。必须要做的事情才是目标。

这的最后一步是让所有人都知道代码审查不是您的主意，他们会留下来，所以每个人都应该努力充分利用它。如果每个人都感到无能为力，也许他们可以被允许查看您的代码？审查”，并认为，如果他们只是将其添加到您的流程中，就会在没有大量时间，精力和精力的情况下发生奇迹。

#9 楼

这可能很苛刻，但是如果没有什么可衡量的，那就不要担心会给出良好的反馈。

但是，在将来，随着您的开发人员开始改进其代码，那时候您将需要给他们很好的反馈。您将要指出代码中的改进之处，并且还要指出对整个团队的好处。当您开始看到改进时，它们也会一样，事情应该慢慢开始出现。

另一件事；可能会有防御气氛，因为他们觉得自己好像没有发言权。为什么不让他们检查彼此的代码？向他们提出具体问题，并设法让他们参与进来。不应该是你与他们对立；这应该是团队的努力。


如果有时间，您会对这段代码有什么改变？
如何改善代码库的这一部分？

现在问，然后问六个月后。这里有一种学习经验。

要点-不要在不需要保证的代码方面给予称赞，而要承认自己的努力并肯定会得到改进。尝试将评论作为团队练习而不是对抗性练习。

#10 楼

没有张力的质量
您已经问过如何找到关于代码的正面评价，但是您真正的问题是如何在解决“严重的质量问题”时避免“ [您的]团队内的压力”。 >将“坏消息与好消息”夹在中间的老把戏可能适得其反。开发人员可能会从中发现它的实质：一种创新。
您的组织自上而下的麻烦
您的老板要求您确保质量。您想出了代码质量标准列表。现在，您希望向您的团队提供积极加强的想法。
为什么要问我们需要做些什么来使您的团队感到高兴？您是否考虑过问您的团队？
听起来您在尽力做到最好。问题不在于您如何传递消息。问题是沟通是单向的。
建立质量文化
必须培养一种质量文化，以便从下而上发展。质量提升得不够快，您想尝试应用《哈佛商业评论》的一些建议。
与您的团队会面。为您希望在团队中看到的行为建模：谦虚，尊重和对改进的承诺。最近遇到的生产问题。我个人认为我没有解决此问题。我认为我最大的错误不是一开始就没有更多的人参与。 [同事]和我仍然对代码质量的管理负责，但展望未来，我们将共同努力进行质量工作。”
从团队中获取有关代码质量的想法。 （白板会有所帮助。）确保您的需求到最后为止。以尊重的方式提供您的见解，并在适当时提出问题。愿意为您的团队感到很重要感到惊讶。保持灵活性，而不会损害业务需求。
（如果您有些尴尬的旧代码，则可以将其淘汰，鼓励大家坦率。最后，让人们知道您编写了它。模拟当其他人受到建筑批评时希望的成熟反应。 ）
协作代码审查
我没有在您所描述的地方工作过，那里有一些高级程序员审查所有代码并进行更正。难怪人们会像您是老师在他们的论文上打上红色标记的那样回答。最好在小组中完成，包括您或其他负责任的开发人员。确保每个人都受到尊重。
同行评审代码的一个重要目标是确保团队的所有成员都能理解该代码。考虑一下您的函数名称不清楚的示例：比起初级开发人员，他们发现函数名称令人困惑，这比高级开发人员的另一项“更正”更容易接受。
质量是一个旅程要做的事情是消除所有认为代码审查是通过/失败的想法。每个人都应该期望在代码审查后进行一些编辑。 （而且您的工作就是确保这些事情发生。）
但是，如果那行不通...
假设您在建立质量文化方面迈出了一大步。您可能仍然无法吸引所有人。如果某人仍不参与质量计划，那么问题是他们不适合团队，而不是你们两个之间存在问题。如果需要从团队中解雇他们，团队中的其他成员将更好地了解原因。

评论

---

注意对等代码审查。我们做了一段时间，直到我们意识到一个初级开发人员对另一个初级开发人员进行了审查，并允许通过它的代码永远不会过去。团队中的两名前辈现在进行审查。

–古斯塔夫·贝特拉姆（Gustav Bertram）
16-10-19在14:16

---

#11 楼

很抱歉，还有一个很长的答案，但是我认为其他人没有完全解决这个问题的人为因素。

有时甚至只是问为什么要以特定方式实施某些事情。当我认为这很糟糕时，我指出我会用另一种方式来开发它。是您将开发人员立即置于防御状态。这个问题本身可能意味着各种情况，具体取决于上下文：您是否太愚蠢而无法想到更好的解决方案？这是您能做到的最好的吗？您是否要破坏这个项目？您是否还在乎工作质量？等等。问“为什么”开发代码的方式只会是对抗性的，这颠覆了您的评论可能具有的教学意图。对抗性的，因为开发人员立即想到的是“我是这样做的……您有问题吗？”再一次，它比需要的更具对抗性，并且使讨论不再改进代码。
示例：
而不是问“为什么选择不为此值使用常量变量？” ，只需声明“该硬编码值应替换为文件XYZ中的常量Constants.h”。该问题表明开发人员主动选择不使用已定义的常量，但是他们甚至可能完全不知道该常量存在。有了足够大的代码库，每个开发人员都不会有很多事情。对于开发人员来说，这只是一个很好的学习机会，使其熟悉项目的常量。
有一条很好的路线可以进行代码审查：您不需要为所讲的内容加糖，不需要将坏消息与好消息夹在一起，也不需要减轻影响。可能是我所处的文化，但是我和我的同事从未在代码审查中互相称赞过-我们开发的无缺陷的代码部分就足够了。您需要做的就是找出您看到的缺陷，并可能给出一个原因（当它很明显/简单时为什么不那么有用）。
好：


”需要更改此变量的名称，以匹配我们的编码标准。“


”此函数名称中的'b'必须大写才能成为PascalCase。“


“此函数的代码未正确缩进。”


“此代码与ABC::XYZ()中的代码重复，应使用该函数相反，“


”您应该使用try-with-resources，以便即使发生错误，也可以确保在此函数中正确关闭内容。 [我仅在此处添加了一个链接，因此非Java用户将知道try-with-resources是什么意思]标准。“


错误：


”我认为您可以通过更改变量名以符合我们的标准来改进此代码“


“也许最好使用try-with-resource来正确关闭此函数中的内容”


”查看此函数中的所有条件。它的N路径复杂度超过了我们标准允许的最大复杂度。“


”为什么缩进这里是2个空格而不是我们标准的4个空格？


“为什么编写一个违反我们n路径复杂性标准的函数？”


在错误的陈述中，斜体部分是人们在希望减轻打击时常用的东西，但是在代码审查中真正要做的只是使您不确定自己。如果您（审阅者）不确定如何改进代码，那么为什么有人应该听您的话？
“好”的陈述是直截了当的，但他们并没有指责开发人员，而是攻击开发人员，他们不是对抗性的，他们也不怀疑缺陷存在的原因。它存在；解决办法。他们当然没有最后一个“为什么”问题那样具有对抗性。

#12 楼

您看到的问题是：开发人员不满意他们的代码受到批评。但这不是问题。问题在于他们的代码不好（显然，假设您的代码评论很好）。

如果开发人员不喜欢他们的代码引起批评，那么解决方案很简单：编写更好的代码。您说您的代码质量有严重问题；这意味着需要更好的代码。

“为什么该方法应该有一个合理的名称，我知道它的作用？”我感到特别不安。他知道它的作用，或者至少他是这样说的，但是我不知道。任何方法都不仅应该有一个合理的名称，还应该有一个名称，该名称应使代码的读者可以立即清楚地知道其作用。您可能想要访问Apple的网站，并查找有关从Swift 2到Swift 3的更改的WWDC视频-进行了大量更改，只是为了使所有内容可读性更高。也许这种视频可以使您的开发人员相信，比他们聪明得多的开发人员会发现直观的方法名称非常非常重要。

另一个令人不安的事是您的同事，她说：“她告诉我自己，在客户提交错误报告后，她知道该错误，但担心开发人员会为此而生气。指出来”。可能会有一些误解，但是如果开发人员生气，如果您向他们指出错误，那就不好了。

#13 楼

我会很不同意您描述的代码审查方法。两名工作人员走进一个“封闭的房间”并受到批评，这使这种对抗性的观念制度化了，这种观念应该避免良好的代码审查。

使代码尽可能地回顾积极的经验对于使其成功至关重要。第一步是消除对抗性评论的概念。使其成为每周或每两周一次的团体活动；确保每个人都知道他们会受到欢迎。订购披萨或三明治或其他任何东西。如果这会激起负面的含义，甚至不要称其为“代码审查”。寻找一些值得庆祝，鼓励和分享的东西-即使只是通过当前的sprint或迭代取得的进展而已。轮流指派领导进行审查。

使这一过程努力为产品和人们服务。好处。每个人都在公共场合接受教练。如果您有一个问题程序员没有“得到它”，则应该私下单独解决它们-切勿在更广泛的团队面前。这与代码审查是分开的。

如果您找不到要提出的“好”东西，那对我来说是一个问题：如果项目正在取得进展，并且人们正在努力，则该进展本身值得庆祝。如果您真的没有发现任何不好的地方，则意味着自上次审核以来所做的一切都是不好的，或者没有比中立更好的了。是真的吗？

关于细节，共同的标准是必不可少的。让每个人都在做什么。并允许将更新，更好的技术集成到您的代码库中。不能保证“老习惯”的行为永远不会以“我们一直那样做”的幌子来消除。

所有这一切的关键是，由于代码审查的实施不当，它们给代码审查带来了不好的效果，被用作贬低经验不足或技能欠佳的程序员的工具，这对任何人都没有用。以这种方式使用它们的经理也不可能是非常有效的经理。良好的代码评审，每个人都是参与者，为每个人（产品，员工和公司）服务。

很抱歉，文章的长度，但由于经常被忽略代码审查，这导致了无法维护的代码的遗留问题，即使允许开发人员，也只有极少数开发人员能够这样做更改为已有多年历史的代码库。我选择不再走这条路。

#14 楼

好的代码的矛盾之处在于它根本不突出，并且看起来非常简单易写。我真的很喜欢这个答案中的台球玩家例子。在代码审阅中，这真的很容易掩盖，除非它恰好是不良代码的重构。如果我正在查看代码，并且浏览了一个非常容易阅读的文件，看起来似乎很容易编写，那么我会快速抛出“我喜欢这里的方法简短明了”或任何合适的方法。

此外，您应该以身作则。您应该坚持也要检查您的代码，并且应该对希望团队进行纠正时的行为建模。最重要的是，您应该衷心感谢大家的反馈。这比您可以提供的任何单方面反馈的影响更大。

#15 楼

听起来真正的问题是，只有两个人在进行所有代码审查，只有您自己认真对待它们，这使您陷入了不幸的境地，承担了很多责任，来自其他团队成员的压力。

更好的方法是在团队中分配进行代码审查的责任，并让所有人参与其中，例如让开发人员选择谁查看他们的代码。这是您的代码检查工具应支持的东西。

#16 楼

我已经亲眼目睹了这种情况的发生，并想提醒您，不要接受情绪智力挑战的答案-它们​​会杀死整个团队。除非您想每年招募，培训和规范新的开发人员，否则与同行建立积极的关系至关重要。毕竟，难道不是为了提高代码质量并培养一种首先提高代码质量的文化而进行这些审查的全部目的？我认为，提供积极的强化确实是您的工作一部分，以将该代码检查系统集成到团队文化中。

听起来，您似乎需要检查您的代码检查系统。现在，从它的声音来看，您审查过程中的所有内容都是主观的，而不是客观的。当感觉某人只是因为他们不喜欢别人而拆开您的代码，而不是因为他们有理由在某些不符合准则的情况下可以引用的理由时，很容易感到受伤。这样，就可以以适合您办公文化的任何方式轻松跟踪和“赞扬”代码质量的积极改进（相对于您的审阅系统）。

技术负责人需要在审查会议之外聚会，并制定《编码指南/代码审查清单》，并且在审查期间必须遵守并参考。这应该是一个实时文档，可以随着流程的发展而更新和完善。这些技术负责人会议还应该在“我们一直做事的方式”与“新的和改进的做事方式”讨论之间进行，而无需对开发人员进行审查，就像他们的代码导致分歧。一旦初步指导原则或多或少地得到了完善，积极增强开发人员的另一种方法是征求他们的反馈，然后对之采取实际行动，并以团队的形式不断发展，这确实使我们加快了开发速度。开始审查板载代码，因此您也不会退休，也不会停下来进行审查。

#17 楼

前提...

程序员是解决问题的人。当出现问题或错误时，我们有条件开始调试。

代码是大纲格式的介质。切换到段落格式的叙述以获取反馈会导致必须断开连接。不可避免地会丢失或误解某些东西。不可避免地，审稿人不是用程序员的语言讲话。

很少会质疑计算机生成的错误消息，也不会将其视为个人冒犯。生成的错误消息。它们旨在捕获程序员和自动化工具遗漏的极端情况以及对其他程序和数据不可避免的副作用。

结论...

因此，可以将更多的代码审查项目合并到自动化工具中，则它们收到的效果会更好。

告诫，为了毫无疑问，此类工具必须每天或每周不断更新，遵守对程序，标准和惯例的每次更改。当经理或开发团队决定进行更改时，必须更改工具以强制执行更改。然后，代码审查员的大部分工作便变成了维护工具中的规则。

示例代码审查反馈...

使用以下技术重写了给定的示例：




主题：


库\ ACME \ ExtractOrderMail类。


/>
原理问题...


TempFilesToDelete是静态的


对GetMails的后续调用会引发异常，因为文件已添加它，但删除后再也不会删除。尽管现在只有一个调用
，但是将来可以通过某些
并行性来提高性能。
TempFilesToDelete作为实例变量将允许并行使用多个对象。




次要问题...


GetErrorMailBody具有异常参数


由于它本身不会引发异常，而是将其传递给ToString，是否有必要？


SaveAndSend名称


将来可能会或可能不会使用电子邮件来报告此事件，并且此代码包含用于存储持久性副本和报告任何错误的一般逻辑。一个更通用的名称将允许这种预期的
更改而无需更改相关方法。一种可能是
StoreAndReport。


，但“注释墙”也会掩盖相邻代码中的错误
。我们在Subversion中仍然有它。也许只是一个
注释，确切地引用了Subversion中的位置？

#18 楼

如果您对现有代码没什么好说的（听起来很容易理解），请尝试让开发人员参与其中的改进。将其他更改（重新命名，重构等）捆绑到同一补丁中。它应该进行修复该错误或添加功能的更改，仅此而已。



这很丑陋，但我想它与我们所有其他讨厌的代码一致。让我们稍后进行清理（理想情况下，在补丁中不进行任何功能更改）。它使您有机会说出为什么认为某事是好事而不是坏事，并且还展示了一个很好地接受建设性批评的例子。一个新功能，很好，它们位于主补丁中。但是，如果您要修复错误，则测试应在先前的补丁程序中进行，并且不能通过，因此您可以演示该修复程序实际上已修复了该错误。 ，或重构内容和时间）应该在工作完成之前进行讨论，这样他们在发现问题之前就不会花很多精力。

如果发现代码无法解决您认为应该输入的内容，也可能与开发人员认为合理的输入不匹配。如果可能的话，也要事先同意。至少对它应该应付的内容以及如何允许它失败的讨论有一些讨论。

#19 楼

我认为假设有一个技术或简单的解决方案是错误的：“当我按我的标准判断他们的工作时，我的同事感到沮丧，并有权执行它。”

记住代码审查不只是讨论什么是好是坏。它们隐含地是“我们正在使用谁的码尺”，是“谁来做决定”，因此（最阴险地）是排名。

如果您不解决这些问题，请在没有遇到的问题的方法将很难。这里有一些很好的建议，但是您已经为自己设定了一项艰巨的任务。

如果您是对的，没有任何回旋余地，指出这是一种攻击：有人搞砸了。如果不是：您的另一所MBA学生却没有。无论哪种方式，您都将成为坏人。您不是网守，而是校对员。获得此协议是一个很难解决的问题[1]。我想给您提建议，但我自己还没有掌握...

[1]这还不能因为人们说“好”或停止战斗而得到解决。关于它。没有人愿意说X对于我们的{智力，经验，人力，截止日期等}不切实际，但这并不意味着实际上涉及到某些特定的X实例...

#20 楼

代码审查应该是相互的。大家都批评大家。让我们的座右铭是“不要生气，要平庸”。每个人都会犯错，一旦人们告诉了如何解决他们的代码的热点，他们就会明白这只是正常的过程，而不是对他们的攻击。了解代码以至于可以指出其弱点，并必须说明弱点可以教给您很多东西。寻找缺陷。但是，您可以在修补程序上大加赞赏。及时性和简洁性都值得称赞。