好的,所以很多代码检查都是相当常规的。但是偶尔会有变化,这些变化会广泛影响现有的复杂,易碎的代码。在这种情况下,验证更改的安全性,缺少回归等所需的时间过长。也许甚至超过了自行开发所需的时间。

在这种情况下该怎么办?合并,希望一切顺利吗? (不主张这样做!)最好的方法可以并且仅尝试发现任何明显的缺陷(也许这是代码审查应针对的最大问题吗?)完全合并和测试,而不是完全比代码审查更好的选择吗?

这不是一个特别的问题,是否应该在代码审查中进行测试。这是一个问题,询问在上述情况下最好的选择是什么,尤其是在紧迫的最后期限,没有可用的全面单元测试套件或单元测试对零碎的代码不可行的情况下。

EDIT :我的印象是,到目前为止,我的短语“广泛影响”已经得到了一些答案/评论,并可能认为这意味着更改涉及大量的代码行。我能理解这是一种解释,但这并不是我的意图。我所说的“广泛影响”是指,例如由于代码库的互连性或连锁效应的范围,回归的可能性很高,而未必一定会引起很大的变化。例如,开发人员可能会找到一种方法,方法是调用一个现有的高级例程,该例程将调用级联到许多较低级别的例程,从而用一行代码修复错误。测试和验证该错误修复程序很容易。手动验证(通过代码审查)所有连锁效应的影响要困难得多。

评论

如何运行测试套件以确保您没有损坏任何东西?

如果没有预先存在的测试套件怎么办? -写一个怎么样?

该测试套件将最终提供帮助。但是同行评审和测试是相辅相成的。我认为将一个彼此替换不是一个好主意。

@MasonWheeler:可能是一次对话,您在那篇文章中特别提到了TDD,使用的假设是我认为没有任何自尊的TDD'er会做,但是我已经双向做了,而且我认为单元测试的好处是不言而喻的。

合并,希望一切顺利吗?这真是个坏主意。

#1 楼

坦率地说,这个问题的前提令人震惊。我们假设易碎,复杂的代码有了很大的变化,并且根本没有足够的时间来正确地对其进行检查。这是您应该花费更少的时间进行审查的最后一个代码!这个问题表明您不仅在代码本身上,而且在管理变更的方法上都有结构性问题。

那么如何处理这种情况呢?首先,请先不做任何准备:


确定复杂性的来源,并进行仔细,严格审查的正确重构,以提高抽象水平。刚入学的新员工应该对代码有所了解,他们对您的业务领域有所了解。
确定脆弱性的来源;这可以通过检查代码本身,检查代码的错误修复历史记录等来实现。确定哪些子系统易碎,并使它们更可靠。添加调试逻辑。添加断言。创建相同算法的缓慢但显然正确的实现,并在调试版本中运行两者并验证它们是否一致。在调试版本中,导致罕见情况更频繁地发生。 (例如,创建一个内存分配器,该内存分配器总是在重新分配时移动一个块,或者总是在页面的末尾或其他任何位置分配一个块。)使代码在面对其上下文更改时变得健壮。现在您不再需要脆弱的代码了;现在您有了找到错误的代码,而不是导致错误的代码。
编写一套自动化测试。显然。
不要做大的改变。进行一系列小的,有针对性的更改,每个更改都是正确的。

但是从根本上讲,您的情况是“我们陷入了技术债务漏洞,每一个复杂的,未经审查的变更都会使我们更加深入;我们应该怎么做?”当您发现自己在那个洞中时该怎么办?别再挖了如果您有太多债务,以致于无法执行诸如检查彼此的代码之类的基本任务,那么您就需要停止增加更多债务并花时间偿还债务。

评论


从我在行业中看到的情况来看,“停止挖掘”通常是快速终止,然后是找到愿意使用铁锹的人。这个答案应该添加一个免责声明,即低码猴子不应该为后果做准备...

–路加·莱伯(Luke A. Leber)
16年6月30日在0:59

@Luke如果管理层或高级开发人员在遇到问题时仍如此努力前进,甚至会考虑终止试图使这种情况变得理智的任何人(好的,撇开公然的不服从命令),那么该公司将步入不可挽回的死亡之路。留给他们吧。

–朱莉娅·海沃德(Julia Hayward)
16年6月30日在9:08

@JuliaHayward您是正确的,但Luke所描述的情况很常见,尤其是在已经产生收入的代码上。确实,继续努力是否值得取决于您。

–欧文
16年6月30日在19:12

@ LukeA.Leber你是正确的。我曾在这些公司工作。我能告诉你的是,死亡行军将需要数年才能完成,但是每个月都会变得越来越糟。 “代码猴子”每个月都会更加悲惨,但是糟糕的经理要花几年的时间才能意识到自己行动的后果……如果有的话。

– JS。
16年6月30日在21:46

@Matt:这个问题的假设是,有人对代码质量足够在意,以建立正式的代码审查系统,而提出该问题的人则担心大型更改对代码质量的影响。如果我们假设没有人关心代码质量,那么可以肯定,我关于确保代码质量的方法的答案不适用,但这不是所要提出的问题!

–埃里克·利珀特
16年7月1日在14:38

#2 楼

代码审查的主要目标之一是提高质量并提供可靠的代码。健壮,因为4眼通常比2眼发现更多问题。而没有编写其他代码的审阅者更有可能挑战(可能是错误的)假设。

在您的情况下,避免同行评审只会增加代码的脆弱性。当然,使用可靠且可重复的测试套件加强测试肯定可以提高质量。但这应该是同行评审的补充,而不是替代。

我认为必须理解和掌握复杂性,而全面同行评审是分享知识并实现这一目标的机会。您在使更多的人了解易碎代码的优点和缺点方面所做的投资,将有助于使其随着时间的推移变得更好。

引述结论:


“如果你想快一点,就一个人走。如果你想走远一点,就一起走”


评论


确实,想象一下如果将“复杂”替换为“冗长”或“样式不佳”或“记录不良”或任何其他负面特征,我们会说“这不是审查的好理由-让我们解决这些问题,以便对其进行审查! ”这没什么不同。

– corsiKa
16年6月29日在9:22

我还要补充一点,如果现在无法审查代码,那么从现在开始六个月就无法维护代码了.....

– corsiKa
16年6月29日在9:23

@corsiKa为什么要等待6个月才能使其无法维护?

–磷虾
16年6月29日在11:59

@krillgar嗯...不是...那只是我从脑海中拔出的一个数字,代表您设置代码到必须再次捡起之间的一段时间...所以,是的

– corsiKa
16年6月29日在15:43

@krillgar:我写了一些“新代码”,签入,去吃午饭,当我回来时,我的“新代码”神奇地变成了“旧代码”。怎么会这样:)

–埃里克·利珀特
16-6-29在17:17



#3 楼

欢迎来到旧软件开发的世界。

您有成千上万,数百万,数千万的代码行。

这些代码行很有价值,因为它们产生了收入流,而替换它们是不可行的。

您的业务模型基于利用该代码库的基础。因此,您的团队很小,代码库很大。为了使人们能够购买新版本的代码或使现有客户满意,需要添加功能。

在理想的环境中,庞大的代码库已经过wazoo的单元测试。您不会生活在一个完美的世界中。

在一个不太完美的世界中,您有预算来解决技术债务-将代码分解为可测试的单元,进行昂贵的集成测试,迭代。

但是,这在不产生新功能的情况下偿还了债务。这与“从现有代码中获取利润,同时对其进行修改以产生升级动力”的商业案例不符。 。但是,在与现有代码进行交互的任何地方,都会暴露出可能的断点。您摆脱了系统中的黑手,实际上弥补了您没有重写的子系统中的怪癖。总是。

你可以做的很谨慎。您可以找到一些您真正理解的代码,并且可以很好地理解其行为以及与系统其余部分的交互。您可以使其现代化,添加单元测试并使其行为更加清晰。

然后找到应用程序其余部分中主要与之交互的部分,并一次对其进行攻击。 >
执行此操作时,您可以改进子系统,添加客户愿意支付的功能。破坏提供业务案例的东西。

但这不是你的问题。您的问题是,“我做的事情很庞大,很可能会打破常规,我该如何遵循最佳实践?”

做大的事情时,确实想做可靠地,与编写错误相比,最终您会花费更多的精力来查找和修复错误。这是软件开发的一般规则:编写东西很容易,使其完美无瑕地工作就很困难。变化就完成了。它是“完成”的,所以您会反驳说“不,这没有完成,看起来就很像”。

如果您有权力和预算,实际花费精力来产生对变更有效的信心,或者干脆拒绝变更。

如果您没有那么多功能,但仍然有一些功能,请尝试坚持要求新系统可以进行单元测试。如果要重写某个子系统,请坚持认为新子系统是由行为明确的小部件和围绕它们的单元测试组成的。您会更深地陷入债务。通过拥有更多易碎的代码和更多的错误来借用程序的未来,以便立即使用该功能并减轻后果。您执行基于扫描的质量检查以发现最严重的问题,而忽略其余问题。从业务角度来看,这有时有时是正确的答案,因为它现在最便宜。债台高筑是一种有效的商业策略,尤其是当桌上有通过破产清算债务(放弃代码)时更是如此。

一个很大的问题是,公司所有者的激励很少与决策者和程序员保持一致。往往存在很大的“交付”压力,而通过(对上级而言)产生几乎看不见的技术债务来这样做,是一个很好的短期策略,有时是中期策略。即使您的上级/利益相关者最好不要承担所有债务也能为其提供最好的服务。

评论


我经历了很多上述令人沮丧的事情。我混合了拙劣的编程实践,移动的目标杆和无情的管理期限,这意味着我们都知道应该发生的事情以及真正发生的事情是两个截然不同的事情

– Ben Hillier
16年7月1日在8:48

这是一个很好的答案,因为尽管其他许多技术都更正确-这在现实世界中被抵消了,而我们所有人都希望生活在一个经过了充分测试,充分记录的世界中-但我们没有。旧版代码,奇怪的实现,误解,不合理的涉众,恶劣的天气.....生活会给您带来糟糕的事情,您将不得不处理它。

–艾伦·汉森(Allan S. Hansen)
16年7月4日在6:12

#4 楼

解决更大的问题,这些问题导致代码审查过于困难。

到目前为止,我发现的是:


没有单元测试套件和代码职责的委托
基本体系的缺乏


#5 楼


您可以发回代码审查,并告诉开发人员将其分解为更小,更多的增量变更集,并提交较小的代码审查。
您仍然可以检查代码的气味,样式和反样式。 ,代码格式标准,SOLID原则等,而不必遍历代码的每个细节。
您仍然可以执行战术代码检查,以进行适当的输入验证,锁定/线程管理,可能的未处理异常等详细信息。级别,而不必了解整个变更集的总体意图。
您可以提供对可能受到代码影响的总体风险领域的评估,并要求开发人员确认这些风险领域已经过单元测试(或要求他编写自动化的单元测试,并将其也提交审查)。


#6 楼


在这种情况下,验证更改的安全性,缺少回归等所需的时间过长。


代码审查不应主要针对正确性。他们在这里是为了提高代码的可读性,可维护性和遵守团队标准。

在代码审查期间发现正确性错误是这样做的一个很好的副产品,但是开发人员应确保将其代码完美工作(包括非回归),然后再将其提交进行审查。

正确性应该从一开始就建立起来。如果一个开发人员无法实现,请让他们与整个团队配对程序或制定计划,但不要将其视为可以事后添加的东西。

评论


同意,但是:代码审查实际上有0的目的,这比代码的可读性,可维护性等更为重要。它们的目的是教育团队有关团队的标准。即使由于代码审查而没有执行任何编辑,它们仍将达到其目标的75%,因为审查会教育代码编写者避免在█的漫长的未来生命周期中再次重复犯相同类型的错误。这个项目,还有下一个...

–乔纳森·哈特利
16年7月1日在16:03



当然,它也可以扮演这个角色,但是我发现结对编程比CR更加有效,可以在新团队成员的入职和早期至中期教育中使用。想一想在练习中一直坐在您旁边的教练与只进行事后评估的老师。根据我的经验,由某人纠正您的“完成”的工作比与某人共同完成的工作更令人沮丧,而且教育程度较低。

– guillaume31
16年7月1日在17:05



@JonathanHartley:在这种情况下,进行代码审查的理由(减去第一)是使开发人员编写不感到羞耻的代码,以免他们在代码审查中展示给其他人:-)

– gnasher729
16年7月5日在23:04

绝对同意上述guillaume31和gnasher729。

–乔纳森·哈特利
16年7月6日在12:30

#7 楼

如果您认为代码审查太难了,因为它更改了脆弱的代码,而几乎不更改就无法更改它,那么您就遇到了问题。但是问题不在于代码审查。问题也不在于单元测试,因为脆性代码无法进行单元测试!如果您的代码是可单元测试的,那么它将被拆分成多个小的独立单元,每个单元都可以进行测试,并且可以很好地协同工作,而这正是您所不需要的!

所以您有一堆垃圾代码(又称“技术债务”)。您能做的最糟糕的事情是开始修复那堆垃圾代码,而不是完成工作,因为那样您将得到更大的垃圾代码堆。因此,第一件事就是让您的管理层全心投入修复并完成工作。否则你不会。在这种情况下,您只是不要碰它。

修复它时,您从代码中提取了一个单元,将其制成具有明确定义和记录良好的行为,为该单元编写单元测试,代码对其进行了审查,并祈祷一切都没有中断。然后对下一个单元执行相同的操作,依此类推。

当您遇到bug时,棘手的问题就来了。在某些情况下,您的代码巢会做错事情,因为事情是如此的脆弱和复杂,事情会出错。提取单位时,其余代码将变得更清晰。 (我有一种情况,在进行一些重构之后,一个函数以“ if(condition1 && condition2 && condition3)crash();”开头,这正是重构之前的行为,只是更清楚了。我然后删除了该行:-)您将看到很明显,这很奇怪和不想要的行为,因此您可以解决它。另一方面,您必须在此处更改现有代码的行为,因此需要谨慎进行。

评论


困难的部分是向业务部门解释:“是的,我们将引入一些错误,但我们将对其进行修复并快速进行修复。现在,稍有耐心将使您获得新功能并在将来更快地修复错误。”

–RubberDuck
16年6月30日在12:06

#8 楼

不幸的是,在代码审查时,除了喝杯咖啡以外,您实际上无能为力。解决此问题的实际方法是解决您积累的技术债务:脆弱的设计,缺乏测试。希望您至少具有某种功能上的质量检查。如果没有,那就总是在祈祷一些鸡骨头。

#9 楼

如果您不满意随附错误/无法正常运行的软件并在以后进行修复,那么V&V的工作应该比开发工作要长!您甚至应该更改它吗?”管理层需要对重新设计和重新实现此代码的成本/风险是否大于修复摇晃的垃圾的成本/风险进行评估。如果是一次性的,则修补它可能会更容易。如果将来可能需要进行更多更改,那么立即采取打击措施以避免将来遭受更多痛苦可能是一个更好的决定。您需要与管理层一起提高这一水平,因为为经理提供良好的信息是您工作的一部分。他们需要做出此决定,因为这是超出您责任级别的战略决定。

#10 楼

根据我的经验,在对相关系统进行任何更改之前,我强烈建议您对代码进行单元和集成测试。重要的是要记住,如今有很多用于此目的的工具,与您所使用的语言无关。

此外,所有工具中的THE工具都可以帮助您创建自己的工具集成测试。是的,我谈论的是容器,尤其是Docker和Docker Compose。它精美地为我们提供了一种快速构建带有基础结构(数据库,mongodb,队列服务器等)和应用程序的复杂应用程序环境的方法。

该工具可用,请使用它们! :)

#11 楼

我不知道为什么还没有提到它,但是这两个是最重要的部分:


您将变更列表拆分为多个较小的变更列表,然后查看其中一个*
如果更改列表的审阅未得出更改列表看起来不错的决定,则您显然会拒绝更改。

*示例:您将库A替换为库B。一个变更表引入了库B,各种不同的变更表将A的用法逐段替换为B(例如,每个模块一个变更表),最后一个变更表被删除库A.

#12 楼


尽力而为,只尝试发现任何明显的缺陷(也许这是应该针对的最多代码审查)?


不要小看潜在的价值代码审查。它们可以很好地检测错误:


查找在测试中难以检测到的错误
查找在测试中难以识别/修复的错误

由于其他原因,它们也很有用:


帮助对团队成员进行交叉培训
帮助确保代码符合其他质量指标,例如帮助确保它是可理解和可维护的,而不仅仅是没有错误


在这种情况下该怎么办?


在最佳/理想情况下在这种情况下,通过代码检查不仅仅意味着“没有明显的错误”:还意味着“显然没有任何错误”(尽管当然您也想对其进行测试)。

如果可以的话”如果无法通过代码检查来验证新的代码库,则将需要进行更广泛的“黑匣子”测试。您可能已经习惯了将代码通过检查后投入生产的开发周期,但是如果代码无法“通过检查”,则您就不能“将其投入生产”,并且需要更长的周期:集成测试,系统测试,alpha测试,验收测试,beta测试等。


没有全面的单元测试套件,或者单元测试不适用于已更改的片段代码


集成,系统和验收测试如何?

无论如何,您可能应该告诉项目经理和产品经理代码几乎肯定是错误的,并且数量不明的错误;并且他们将“得到他们检查的内容”,而不是仅仅得到“他们的期望”-即代码质量并不比他们的测试好(因为代码质量一直没有,并且不能通过代码检查来保证) 。

他们可能应该将该消息传递给客户或用户,以便他们进行Beta测试(如果他们愿意成为早期采用者),或者使用较旧的版本,直到新版本退出Beta(如果不是)。

#13 楼

大量的代码被编写和合并,而没有进行适当的代码审查。它可以工作。有一个原因将其称为代码气味而不是“破损的代码”或某种影响。缺少代码审阅是一个警告信号,而不是厄运的预兆。

解决此问题的方法是,没有一种解决方案可以适应我们可以打包成StackExchange样式答案的所有情况。软件开发社区的强烈共识是,代码审查是至关重要的“最佳实践”,在这种情况下,它已被跳过。您的发展不再局限于“遵循所有最佳实践”的狭窄范围。您将需要找到自己的方式。

什么是“最佳实践”?一经发现,它就会成为人们通常认为使代码更好的一组实践。他们编写正确的代码吗?哎呀!互联网上到处都是遵循“最佳实践”的公司的故事,这些故事使自己陷入困境。关于“最佳实践”的一个更好的观点也许是,它们是软件界的“解雇”解决方案。我对您的公司,您的项目,您的团队一无所知,并且能够嘲笑“最佳实践”作为对您有帮助的事情。它们是一般的“请勿伤害”建议。

您明显偏离了该计划。幸运的是,您认识到它。做得好!他们说知识是成功的一半。如果是这样,那么知名度就超过一半!现在需要一个解决方案。从您的描述中可以很明显地看出,您所处的业务环境已经发展到了“去做代码审查,这是最佳实践”这一无聊的建议不会减少的程度。为此,我建议使用有关软件最佳实践的关键规则:


没有软件开发最佳实践能胜过业务需求。


坦白说,他们付钱给您,业务的生存通常比软件的质量重要得多。我们不愿意承认这一点,但是如果完美写作软件因其维护完美写作软件的努力而陷入公司内部,那么它就毫无用处。

那么,您去哪儿了?跟随力量的踪迹。您已经指出,由于某些未说明的原因,对某些任务进行代码审查是不合理的。以我的经验,这个原因总是短暂的。总是“没有足够的时间”或“没有足够的钱来使您在花时间时保持薪水流动”。这是生意;没关系。如果容易的话,每个人都会做。沿着力量的轨迹向上走,找到可以帮助您理解为什么不进行代码审查的管理人员。语言很难,而且通常会有一项法令从高层管理人员那里滴下来,并被扭曲。问题的解决方案可能隐藏在这种失真中。

答案一定是特定的情况。这类似于试图预测抛硬币是正面还是反面。最佳做法是将其翻转100次,预期将大约有50头和50尾,但您没有时间将其翻转1次。在这里,您的情况的细节很重要。您是否知道硬币通常会以大约51%的时间被抛掷的相同方向着陆?您是否花时间观察抛硬币之前的方向?可能会有所作为。

可能会用到的一种通用解决方案是尝试找到一种方法来制定代码审查过程,并使其成为一项非常低成本的工作。代码审查过程的大部分成本是,每个人在执行代码审查时都会全力以赴100%。之所以如此,是因为一旦完成代码审查,代码就会受到祝福。也许您可以将代码放在不同的分支中,并与主干上的开发并行进行代码检查。也许您甚至可以对其进行设置,以便该软件为您进行测试。也许您在一个业务环境中,您的客户可以与旧客户并行运行“新”代码,并让他们比较结果。这将客户变成一堆用例创建设备。

所有这些正在运行的“ maybes”的关键在于,您应努力使代码轻松分解。通过在不太关键的项目中使用它们,您也许可以“证明”部分代码,而无需依赖正式的代码审查。如果更改较小,即使更改的总和太大而无法进行同行评审,则更容易做到这一点。

通常,寻找针对您的项目,公司,您的解决方案球队。通用答案是“最佳实践”。您没有使用这些,所以这次您应该寻找针对此问题的更多定制解决方案。这是生意。如果一切都按照我们一直期望的方式进行,IPO就会很容易分配价值,不是吗!

如果要替换代码审查很麻烦,请记住,从来没有一个代码可以在代码审查中被证明有效。*代码审查所做的只是使您对代码充满信心,并且有机会进行更正在他们成为问题之前。可以通过其他方式获得代码审查的这两种有价值的产品。代码审查只是具有特别出色的公认价值。一个合格的C ++编译器,将完全按照文档中的说明进行操作。

评论


您的答案表明,“自动证明系统”会自动检查L4的源代码。实际上,它审查了人为证明L4正确性的证明。该证明需要花费数年时间才能完成。尽管如此,从这项工作中可以学到很多有关如何编写正确代码的知识。 (请注意,这并不是笔试,而是机器可读的证明,实际上是“导入”了整个源代码及其原因。请参阅ssrg.nicta.com.au/publications/nictaabstracts/3783 .pdf)

– Artelius
16年7月5日在5:13

#14 楼

正如@EricLippert在其出色的回答中指出的那样,这种变化需要更多的关注,而不是更少的关注。如果您意识到要进行的更改会变成这样的更改,那么有一些策略可能会有所帮助:


经常进行版本控制。审核可以逐个提交进行,当提交量较小时,审核可能更容易理解。
请确保您尽可能清楚地说明每次更改的原因。使用结对编程进行这种更改。三眼关注该问题,而不是两眼可以帮助避免正常情况下可能会遗漏的问题,而在工作时进行三对可以帮助您改善对您认为显而易见但对代码不那么明显的任何注释比您认为的要好,这反过来将对以后的审阅者有所帮助。 (a)减少开发过程中的错误和(b)改进文档的帮助实际上意味着尽管有更多的人参与,但花费的工时却更少。

#15 楼

更多答案正在解决您如何达到这一点。他们中的许多人都提出了一些建议来纠正这种情况,但是我想将我的答案提供给我一个简短的答案。

当代码审查“太难了”时该怎么办?


返回主代码分支
为已重构的功能编写测试(例如功能测试)
让测试通过
合并对“难以测试”的代码进行测试
测试仍然通过吗?

是的

您的开发人员很棒!欢迎大家回来!

(或那些没有长大的人在美国电视上观看《辛普森一家》的人:如果测试通过了,请跳过尝试看一下差异并由开发人员领导您正在浏览变更)



继续重构并添加测试覆盖范围,直到测试通过。

评论


猫回来意味着什么?

–JDługosz
16年6月30日在18:28

@JDługosz现在是Simpsons参考。

–类风湿
16年7月4日在23:47

我不明白

–JDługosz
16年7月5日在1:07

体操教练卢加什(Lugash)有没收其学生的猫和狗的习惯,只有在学生完成身体任务后才将其归还。 simpsons.wikia.com/wiki/卢加什

–马克·麦克拉伦
16年7月5日在10:35

#16 楼

像乘法一样,代码复审应用于零时会给出零结果。在这种情况下,它不会增加价值,而在其他大多数情况下,它却不会增加价值。

您需要使用的代码设计得太糟糕,无法在进一步的开发中受益于代码审查过程。使用代码审查过程来重构或重新开发它。

也可能是代码仍然可以忍受,但任务不好。它太宽了,应该以较小的增量完成。

评论


@Downvoter,代码审查不能替代不良设计,并且无论如何尝试应用它通常会导致变更从未得到批准,因为审查者不了解垃圾中的这些垃圾变更。抱歉破坏您的视野。

– h22
16年7月1日在19:01