在审查过程中，我该如何在技巧上建议改进其他人设计不良的代码？

#1 楼

再次检查您的动机。如果您认为应该更改代码，则应该能够阐明您认为应该更改的某些原因。这个原因应该比“我本来会做得不同”或“丑陋”更为具体。如果您不能指出所建议的更改所带来的好处，那么花时间（又称金钱）来进行更改就没有多大意义了。
项目中的每一行代码都必须是一行保持。代码应该只要能够完成工作并易于理解就可以，并且不再需要。如果您可以在不牺牲清晰度的情况下缩短代码，那很好。如果您可以在提高清晰度的同时做到这一点，那就更好了。
代码就像具体的一样：坐了一段时间后，更改起来更加困难。如果可以的话，请尽早建议您进行更改，以使更改的成本和风险都最小化。
每次更改都会花费金钱。重写有效且不太可能需要更改的代码可能会浪费很多精力。将注意力集中在更容易更改的部分或对项目最重要的部分。
表单遵循功能，有时反之亦然。如果代码凌乱，则很有可能还包含错误。寻找这些错误，并批评有缺陷的功能，而不是代码的美观。提出改进建议，以使代码更好地工作，并使代码的操作更易于验证。
区分设计和实现。具有a脚界面的重要类可以通过像癌症这样的项目传播。这不仅会降低项目其余部分的质量，而且会增加修复损坏的难度。另一方面，具有精心设计的接口但糟糕的实现的类应该没什么大不了的。您始终可以重新实现该类，以获得更好的性能或可靠性。或者，如果它可以正常工作并且足够快，则可以放心使用它，因为它的碎片被很好地封装了。

总结以上所有几点：确保建议的更改增加价值。

#2 楼

有一个通过重构增加价值的最佳选择。更改需要完成三件事：


改进可能更改的代码
增加清晰度
花费最少的精力

注意事项：


我们知道，干净的代码编写和维护成本较低，并且使用起来更有趣。您的工作是将该想法出售给公司中的人员。像推销员那样思考，而不要像傲慢的脾气一样（即不像我）。
你赢不了，只能少花些钱。
专注于增加真实价值-不仅仅是美。我希望我的代码看起来不错，但有时必须更多地接受那些廉价的事情。
找到最佳位置的一个好方法是遵循“童子军原则”-在编写代码区域时，请始终保留它形状比您发现的要好。
有一点改善总比没有改善要好。
充分利用自动化工具。例如，仅清理一点格式的工具可能会带来很大的不同。
出售其他想法，它们可以偶然地提高代码的清晰度。例如，单元测试鼓励将大型方法分解为较小的方法。

#3 楼

如果代码运行时没有严重的错误，并且临近主要的截止日期（例如，损益表或公司PR），那么提出需要进行重大更改的改进为时已晚。甚至代码的改进也会对项目部署造成风险。改进的时间早于该项目，当时有更多时间投资于代码库的未来健壮性。

#4 楼

代码审查有3个目的：


检查错误
检查代码可以改进的地方
编写代码的人的教学工具。

评估设计/代码质量当然与＃2和＃3有关。

就＃2而言：




制作非常清楚，建议的变更与修复成本之间的关系是什么。作为任何业务决策，这应该与成本/收益分析有关。

例如“ X的设计方法将显着减少进行更改Z时发生错误Y的可能性，而且我们知道这段代码每2周经历一次类型Z的更改。处理由于错误Y +查找错误+修复和释放不提供下一组功能的修复成本+机会成本为$A；而现在清理代码和机会成本（例如，延迟交付或功能较少的价格）的成本为$B。宁愿让您的团队负责人/经理-评估$A与$B并做出决定。


这将帮助精明的团队领导有效地进行管理。例如，他们将使用全部信息做出合理的决定
这将（特别是如果您说得好）会提高您的地位-例如，您是一个足够聪明的人，可以看到更好的设计的好处，并且足够聪明，不会在没有权衡商业考虑的情况下虔诚地要求它。
在李错误Z发生的重大事件，您将获得更多建议，利用下一组建议。



＃3：


非常清楚地从“这是最佳实践，如果我们可以节省资源，请务必修复-请参阅附加的优点/缺点分析”，对设计的改进（附加上面＃2中描述的内容）与“这些是我认为可以帮助您提高代码健壮性的常规准则，因此您可以更轻松地维护代码的可选更改。请注意该措辞-并不是“使代码像我想要的那样”-而是“如果这样做，您将获得a，b，c的好处”。语气和方法很重要。

#5 楼

选择战斗，如果快要截止期限了，那就什么也不做。下次其他人正在审阅或维护代码时，他们仍然遇到麻烦，然后以这样的想法与他们联系：作为一个团队，您在审阅代码时应该更加专注于代码质量，这样以后就不会有太多麻烦了。

他们应该在做额外工作之前先看清价值。

#6 楼

我总是以“我愿意”开始我的评论，这表明我只是很多观点之一。

我也总是包含一个原因。
”由于可读性，将其限制为一个方法。“

我对所有内容都发表了评论；大和小。有时，我对更改进行100条以上的评论，在这种情况下，我还建议使用结对编程，并表示自己是一名边锋。

我出于某种原因试图建立一种通用语言。可读性，DRY，SRP等。

我还创建了一个有关“干净代码和重构”的演示文稿，向我的同事解释了原因并展示了如何演示。到目前为止，我已经举行了三次，我们正在使用的一家咨询公司要求我再次为他们举行。

但是有些人还是不会听。然后我就离开了排名。我是设计负责人。代码质量是我的责任。

请注意，我非常乐意拒绝我发表的任何评论；出于技术原因，截止日期，原型等原因。我还有很多关于编码的知识，并且会一直听取原因。
哦，最近我提议给第一个午餐者买午餐我的团队提出了一个不重要的变更，对此我没有任何评论。 （嘿，你也必须玩得开心。:-)

#7 楼

[此答案比最初提出的问题要宽一些，因为这是代码审查中许多其他问题的重定向。]

这里有一些我认为有用的原则：

私下批评，公开赞美。让某人一对一地了解代码中的错误。如果他们做了出色的工作或执行了没人要的任务，请在小组会议上或在抄送给团队的电子邮件中称赞他们。

分享您自己的错误。我与学生和初级同事分享了我的灾难性第一次代码审查（收到）的故事。我还让学生知道我这么快就发现了他们的错误，因为我已经在自己之前做到了。在代码审查中，结果可能是：“我认为您在这里错误地定义了索引变量。由于时间错误，我总是检查以确保数据中心失效。” [是的，这是一个真实的故事。]

请记住要发表正面评论。简短的“不错！”或“巧妙的把戏！”可能会成为初级或不安全程序员的一天。

假设另一个人很聪明，但有时会很粗心。不要说：“如果您实际上没有返回值，您如何期望调用者获得返回值？！”说：“好像您忘记了退货声明。”请记住，您在早期就编写了糟糕的代码。正如某人曾经说过的那样：“如果您一年前不对自己的代码感到羞耻，那么您就不会学习。”

为不在您工作场所的朋友节省了嘲讽/嘲笑。如果代码非常糟糕，请在其他地方开玩笑。 （我发现与一位程序员程序员结婚很方便。）例如，我不会与同事分享以下漫画（或本漫画）。

#8 楼

您的问题是“如何对设计不良的代码进行代码审查？”：

答案很简单。讨论代码的设计以及设计有缺陷或不符合要求的方式。如果您指出有缺陷的设计或“不符合要求”，那么开发人员将被迫更改其代码，因为它没有执行所需的操作。

如果代码为“功能上足够”和/或“符合规格”和/或“符合要求”：

如果您是此开发人员的同辈，您将没有任何直接的能力可以让您“告诉他”进行更改。

还剩下两个选择：


您必须使用自己的个人“影响力”（“权力”的一种形式）间接）和/或您具有“说服力”的能力
参与组织“代码处理”小组并开始将“代码维护”作为更高的优先级。
硬着头皮，学习如何更快/更流畅地阅读糟糕的代码，这样您就不会在糟糕的代码上挂断电话（听起来像在遇到糟糕的代码时一直挂断电话或放慢脚步）。

这也将使您成为一个更强大的程序员。
当您处理糟糕的代码时，它将使您更正糟糕的代码。
这也将使您可以处理更多项目，因为许多项目的代码功能很差，但是功能很差。


以身作则。使您的代码更好...但是不要尝试成为完美主义者。

因为那样，您将成为“无法按时完成任务的慢家伙，总是在批评，并认为自己更好。比其他任何人都好”。



我发现没有银弹。您必须同时使用这三种方法，并且必须在使用这三种方法时发挥创造力。

#9 楼

我有时会发现自己的同伴代码混乱不堪
，但设计杂乱，对未来的维护几乎没有顾虑，
尽管它是有效的并且几乎没有bug。


此代码已完成。在某些时候，重新设计的成本太高，无法证明其合理性。如果代码已经可以正常运行且几乎没有错误，那么这将是不可能的。建议一些方法以在将来进行清理并继续。如果/将来代码中断，请重新评估重新设计的价值。它可能永远不会中断，这将是很棒的。无论哪种方式，您都在赌博不中断的合理意义上，因为现在或以后的成本都是一样的：冗长，糟糕的重新设计。

将来您需要做的是更紧密的开发迭代。如果您能够在所有消除臭虫的工作都投入之前查看此代码，则建议重新设计是很有意义的。最后，除非以根本上无法维护的方式编写代码，并且可以肯定地知道在发布后不久就需要更改代码，否则进行大型重构永远是没有意义的。

给出选择在两个选项（重构还是不重构）之间，考虑一下听起来更明智的销售方式：


嘿老板，我们按计划进行了所有工作，但是现在我们
将要重建很多东西，以便我们将来在
中添加功能X。


或



老板，我们准备释放。如果我们必须添加功能
X，则可能要花几天的时间。


如果您说了一个，老板可能会说：


谁说了功能X？


底线是，如果您不能在便宜的时候（早期迭代）纠正某些缺陷，有时需要付出一些技术债务。随着接近完成的功能和截止日期，进行高质量的代码设计会减少收益。

#10 楼

当一小勺糖帮助药物消退时，错误的句子可以简洁地表达出来-没有20件事出现了错误-我将以一种形式表示我没有利益，没有自我投资我想要的东西被听到。通常是这样的：


我想知道这样做是否更好...


或


......有意义吗？


如果原因很明显，我不作说明。这使其他人有机会承担该建议的某些知识，例如：

“是的，这是个好主意，因为<您显而易见的原因>。”

如果改善是相当明显的，但不至于让我看起来像个白痴，没有想到它，并且这样做的原因反映了与听众共享的价值，那么有时我什至不建议这样做，而是：

我想知道是否有办法... <这里的共享价值声明>

这仅用于与真正敏感的人打交道-与我的大多数同龄人一样，我就让他们吃吧！

#11 楼

代码审查并不一定总是针对改进。

项目结束时的审查似乎就是这样，以便每个人都知道从哪里发现错误（或发现错误）。为了设计更好的项目，可以将其用于以后的重用）。无论审阅的结果如何，根本就没有时间进行任何更改。

要真正进行更改，您需要在项目中更早地讨论代码并进行设计-代码容易得多仅在讨论可能的方法时才改变。

#12 楼

万一设计糟糕透顶，您的重点应该放在最大化封装上。这样，就可以用设计更好的类替换各个类/文件/子例程。

着眼于确保组件的公共接口设计合理，并仔细隐藏内部工作原理。同样，数据存储包装器也是必不可少的。 （存储的大量数据可能很难更改，因此，如果“实现流血”进入系统的其他区域，则很麻烦）。

一旦遇到障碍在组件之间，将注意力集中在最有可能引起重大问题的组件上。

重复直到截止日期或系统“完善”为止。

#13 楼

与其直接对某人的代码进行直接批评，不如总是在代码审查期间在我们的注释中起到协同作用。

我遵循的一种方法是


它会
以这种方式编写将使其运行更快。
如果您执行“ this”，“ this”和“ this”，您的代码将更具可读性
/>
即使您的截止日期临近，此类评论也会得到认真对待。并且可能会在下一个开发周期中实施。

#14 楼

问题中有两个值得注意的问题，即机智部分和最后期限。这些是截然不同的问题-第一个是沟通和团队动力的问题，第二个是计划和优先级的问题。

切实地。我认为您想避免刷牙的自我和负面的评论。一些建议：


对编码标准和设计原则有共同的理解。
不要批评或评论开发人员，只批评代码。避免使用“您”或“您的代码”这个词，而要谈论与所有开发人员分离的正在审查的代码。
将您的骄傲放在完整代码的质量上，以便使评论注释有助于改善最终结果表示赞赏。

建议改进，而不是需求。如果您不同意，请始终进行对话。如果您不同意，请尝试理解另一种观点。
让评论同时进行。即让您审核的人接下来查看您的代码。没有“单向”评论。

第二部分是优先级。您有许多改进建议，但是由于临近截止日期，因此只有时间申请一些建议。

首先，您要避免这种情况的发生！您可以通过执行连续的增量审核来做到这一点。不要让开发人员在功能上工作数周，然后在最后一刻将其全部复习。其次，代码审查和实施审查建议的时间应作为任何任务的定期计划和估算的一部分。如果没有足够的时间进行适当的审查，则计划中出现了问题。

但是假设在此过程中出了点问题，您现在面临许多评论，而您没有时间实施所有评论。您必须确定优先级。然后进行更改，如果您推迟进行更改，那么这些更改将是最困难，最冒险的更改。

源代码中标识符的命名对于可读性和可维护性极为重要，但将来更改也很容易且风险很小。与代码格式相同。所以不要专注于这些东西。另一方面，公开暴露的接口的健全性应该是最高优先级，因为它们在将来真的很难改变。持久性数据很难更改-如果您首先开始在数据库中存储不一致或不完整的数据，将来真的很难修复。

单元测试所涵盖的区域是低风险的。您以后可以随时修复这些问题。没有但可以进行单元测试的区域的风险要低于不能进行单元测试的区域。

假设您有很多代码，没有任何单元测试，并且各种代码质量问题，包括对外部服务的硬编码依赖。通过注入此依赖关系，可以使代码块可测试。这意味着您将来可以添加测试，然后再解决其余问题。使用硬编码的依赖项，您甚至无法添加测试。因此，请首先进行此修复。

，但是请尽量避免在这种情况下最终陷入困境！

#15 楼

代码审查必须与文化和开发周期相结合才能起作用。在功能X的开发结束时安排大型代码审查不太可能起作用。首先，进行更改将更加困难，并且有人可能会感到尴尬-对该活动产生阻力。

您应该提早提交和频繁提交，并在提交级别进行审核。有了代码分析工具，大多数审核将很快进行。诸如FindBugs和PMD之类的自动代码分析/查看工具将帮助您摆脱大量的设计错误。但是，它们不会帮助您解决体系结构级别的问题，因此您必须具有可靠的设计并根据该设计判断整个系统。

#16 楼

提高代码审查的质量。

除了正在审查的代码的质量之外，还有代码审查本身的质量：


所提议的更改是否真的比现有的有所改进？
还是只是见仁见智？
除非有非常明显的内容，否则审阅者是否已正确解释，为什么？
花了多长时间？ （我看到审查持续了几个月，而开发人员则负责解决所有众多合并冲突）。
语气？

接受高质量的代码审查要比接受质量好的审查容易得多一些自我怀疑的自我修饰。