首页 > 其他分享 >大厂怎么做Code Review?

大厂怎么做Code Review?

时间:2023-01-22 14:31:14浏览次数:40  
标签:Code Review 评审 问题 正确性 大厂 CR 团队 代码


发现坏味道的实践,就是Code Review:对计算机源代码系统化地审查,常用软件同行评审的方式进行,其目的是在找出及修正在软件开发初期未发现的错误,提升软件质量及开发者的技术。团队对 CR本身的理解有差异,有的团队:

  • 在一个完整的开发周期结束之后
  • 每周一次
  • 每天都要

CR是一个沟通反馈的过程

为什么要做 CR?

没人保证自己写出来的代码没问题,而规避个体问题的主要方式就是使用集体智慧,团队的力量,这是从个体的角度在看问题。

看待 CR还有一个团队视角:CR的过程,也是一个知识分享,保证一些细节的知识不再是隐藏在某一个人的头脑中,而是摆在团队。

无论是从哪个角度看 CR,它的本质,就是沟通反馈的过程。
把对这段代码的理解分享给你,你把你对这段代码的想法共享给我。有人给出代码实现的知识,有人贡献技术理解。如果我们理解了CR是个沟通反馈的过程,那就可以把沟通反馈的一些原则运用到CR。

沟通反馈,希望沟通要尽可能透明、及时。把这样的理解放到 CR中,就是要尽可能多暴露问题,尽可能多做 CR。

暴露问题

要发现啥问题?如果泛泛地回答,那自然就是代码实现中的各种问题。细化,做CR时,我们可以从下面几个角度来看代码:

  • 实现方案的正确性
  • 算法的正确性
  • 代码的坏味道

实现方案

应该是设计评审关注的内容,但在实际工作中,并不是所有团队都能够很好地执行设计评审,而且设计评审有时也关注不到特别细的点。所以,一些实现方案问题只能延迟到CR才发现。

批量处理的 REST 接口

接到请求经过一些处理之后,它会调用另外一个服务,因为这个服务只支持单一的请求,所以,REST 接口只能一个个地向这个服务发送请求。

若一切正常,这个接口没问题。但若在处理过程中出现失败,没有把所有的请求发给另一个服务,这个接口的行为是什么样呢?
需要客户端重新发起请求,还是服务端本身重新调用接口?
如果是服务端负责重试,那么,这个方案本身没有任何重试机制,即一个请求一旦出错,它就丢了,业务不能顺利地完成。

显然,一般人只考虑正常情况,而没有考虑失败场景。把它做成一个完整的方案,很可能还需要做一个后台服务,善后未能得到有效处理的任务。这就不是代码调整,而是设计方案调整。

这是发量多程序员写程序经常会出现的问题:正常情况一切顺利,异常情况却考虑不足。

算法正确性

嵌套代码,对于循环语句,我们要把处理一个元素的代码提取出来。不过,这有时候也会带来一些意外惊喜。

某段代码就是把循环里对于一个元素的处理拆了出去。
但对于每个元素的处理,都要每次查询一次DB,得到相应元素,修改后再回存。

于是,每段代码单独来看,都是对的,但合在一起就有极大性能问题,本可以通过一次查询解决的问题变成N次查询,耗费大量网络 I/O!

有人时常觉得自己项目的代码太慢,代码最多只是看起来写的还不错,一眼难看问题。看了半天,在一个遍历图像像素点的循环里发现了一个图像复制的代码,即每循环一次,都要把整个图像复制一遍,代码慢就很正常啊。

如果这是一个面试,相信每个人都能尽力高性能地解决问题,但在日常业务开发,就难免敷衍了事,代码和人,有一个能 run 就行了。

实现方案的正确性至少还有人评估,算法的正确性最终影响性能,也会有性能指标监控,团队也都会关注。
但代码坏味道最容易被忽略,因为很多互联网团队疲于奔命一直有写不尽的需求,对于坏味道的标准太低了!

CR频率

和人的体质一样,不同团队的频率不同,最糟糕的肯定是不评审,闭眼睛勇往无前,这种外包级团队就不管了。常见的评审频率是每个迭代评审一次,也有每次周会评审。

推荐提升评审的频率,比如,每天一次。
评审周期过长,导致堆积太多问题,让人产生无力感,根本也改不动了。
遇到实现方案存在问题的,几乎要重写,影响项目进度。

提升评审频率,评审的周期就会缩短,每个周期内写出来的代码就是有限的,人就有心力去重构。

如果把CR推至极致,就是有个人随时随地来做 CR。极限编程的理念,就是把好的实现推向极致,而 CR的极致实践就是结对编程。结对编程就是两个人一起写一段代码:

  • 一个人主要负责写
  • 一个人则站在用外部视角保证这段代码的正确性

好的结对编程对两个人的精力集中度要求是很高的,两个人一起写一天代码其实是很累的一件事,不过,也正是因为代码是两个人一起写,代码质量会提高很多。
对于团队中的新人提升也极大。这种高强度的训练和反馈,本质上就是一种刻意练习,而刻意练习是一个人提升最有效的方式。
虽然现实里多数团队根本没有条件做大规模结对编程。但对个体来说,创造一些机会与高手一起写代码也是很好的。即便不能一起写,去观摩高手写代码也能学到很多东西。再退一步,实在身边没有机会,去网上看看高手写代码也是一种学习方式。

总结

对于大部分人,CR只是一个发现问题的过程,本文讨论的是CR是一个沟通反馈的过程。站在沟通反馈的角度,尽可能多地暴露问题,尽可能多地做CR。

CR可以从实现方案正确性、算法正确性和代码坏味道的角度去发现问题。
CR的频率是越高越好,频率越高,发现和解决问题的难度越低,团队越容易坚持下去。

如果把CR推向极致就是随时随地做CR,就是结对编程。

CR暴露的问题越多越好,频率越高越好。


标签:Code,Review,评审,问题,正确性,大厂,CR,团队,代码
From: https://blog.51cto.com/u_11440114/6021499

相关文章

  • B. Emordnilap【Codeforces Round #845 (Div. 2) and ByteRace 2023】
    B.Emordnilap原题链接Apermutationoflengthnisanarrayconsistingofndistinctintegersfrom1toninarbitraryorder.Forexample,[2,3,1,5,4]isape......
  • A. Everybody Likes Good Arrays!【Codeforces Round #845 (Div. 2) 】
    A.EverybodyLikesGoodArrays!原题链接Anarrayaisgoodifforallpairsofadjacentelements【相邻元素】,aiandai+1(1≤i<n)areofdifferentparity【奇......
  • [LeetCode] 684. Redundant Connection
    Inthisproblem,atreeisan undirectedgraph thatisconnectedandhasnocycles.Youaregivenagraphthatstartedasatreewith n nodeslabeledfrom......
  • leetcode笔记——328周赛
    1.二维前缀和,二维差分304.二维区域和检索-矩阵不可变-力扣(LeetCode)二维前缀和怎么处理,注意加减的下标 2.2537.统计好子数组的数目-力扣(LeetCode)首先看到子数......
  • AtCoder Beginner Contest 286
    A-RangeSwap(abc286a)题目大意给定长度为\(n\)的数组\(a\)和\(p,q,r,s\),交换\(a[p..q]\)和\(a[r..s]\)并输出交换后的数组\(a\)。解题思路模拟即可。神奇......
  • 【双指针】LeetCode 409. 最长回文串
    题目链接409.最长回文串思路遍历字符串过程中统计字符出现个数,如果达到2则说明可以放到回文串的两端,需要result+=2。遍历完之后的回文串如果长度小于s,说明s中存......
  • Educational Codeforces Round 1 个人总结A-E
    EducationalCodeforcesRound1A.TrickySum数学,求\(1\dotsn\)的和减去小于等于n的二次幂乘2之和LLf[40];voidsolve(){ LLn; cin>>n; LLans=n+n*(n-1)/2;......
  • vscode配置git.码云
    1.先安装好Git配置用户邮箱gitconfig--globaluser.name"lzl"gitconfig--globaluser.email"30785*****@qq.com"查看git全局配置gitconfig-lgit生成......
  • LeetCode.541 反转字符串II
    1.题目给定一个字符串s和一个整数k,从字符串开头算起,每计数至2k个字符,就反转这2k字符中的前k个字符。如果剩余字符少于k个,则将剩余字符全部反转。如果剩余字符小......
  • LeetCode.面试题02.05-链表求和-题解分析
    题目来源面试题02.05.链表求和题目详情给定两个用链表表示的整数,每个节点包含一个数位。这些数位是反向存放的,也就是个位排在链表首部。编写函数对这两个整数求和,并......