首页 > 其他分享 >代码质量与技术债系列分享之一 - 如何做好 Code Review

代码质量与技术债系列分享之一 - 如何做好 Code Review

时间:2024-03-29 11:59:17浏览次数:36  
标签:Code const Review return CR import 分享 data 代码

TL;DR

Code Review 速查手册

参考资料

https://composity.com/post/too-busy-to-improve
https://commadot.com/wtf-per-minute/
https://dl.acm.org/doi/10.1145/3585004#d1e372
https://google.github.io/eng-practices/review/reviewer/standard.html
https://book.douban.com/subject/35513153/
https://zhuanlan.zhihu.com/p/549019453

名词解释

CR: Code Review
CR:代码审查
CL: Stands for "changelist", which means one self-contained change that has been submitted to version control or which is undergoing code review. Other organizations often call this a "change", "patch", or "pull-request".
CL:代表“变更列表”,表示已提交到版本控制或正在进行代码审查的独立更改。其他组织通常将其称为“更改”、“补丁”或“拉取请求”。
LGTM: Means "Looks Good to Me". It is what a code reviewer says when approving a CL.
LGTM:意思是“对我来说看起来不错”。这是代码审查员在批准 CL 时所说的。

CR意义

灵魂拷问:为什么我们接手的每个代码库都如此难以维护?

image.png
重要原因之一:Code Review 执行不彻底
万能借口:太忙!

  • 疲于应付眼前

  • 不可见,意识不到

  • CR 非功能性开发

  • CR 不是当务之急,没有眼前收益

  • 危害被低估,忽视“复利”的威力(非线性)

意义

现代代码评审【modern Code Review】,业内认为有效的、基于最佳实践的质量保证工作流,可通过人工审代码降低风险、增强可维护性和提升研发效率,同时可以有效提升个人和团队技术能力。更是一种对代码精益求精、追求极致的态度、是“工匠精神”的一种体现。

CR原则

  • 只要CL可以提高整体代码健康状态,就应该倾向于批准合入,即使CL并不完美

  • 基于技术事实和数据的沟通

    • 基于技术事实和数据否决个人偏好和意见

    • 软件设计问题不能简单归结为个人偏好

  • 解决冲突:不要因为无法达成一致而卡壳

  • 善用工具

    • 基于Lint、公司代码规范等工具

    • 大模型辅助

发起CR

发起前的准备

  • 推荐自己做一个 checklist

  • 把自己当作 Reviewer 来对自己的代码进行 CR

  • 预估代码可能出问题的地方

  • 进行充分自测,保证代码可运行

  • 不要指望别人帮你找出问题

checklist 可参考Code Review 速查手册

利用自动化工具进行前置检查

  • 单元测试检查

  • 新增单元测试检查

  • 方法行数过多

  • 圈复杂度过高

  • 代码规范检查

  • lint 检查

  • 体积监控

建议平均时长不要超过10分钟, 所以 e2e,性能检查等建议不阻塞发起MR流程

合理的规模

image.png
image.png
https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/

  • 一次评审200LOC为佳,最多400LOC

  • 评审量应低于500LOC/小时

  • 一次评审不要超过60分钟

  • 采用轻量级评审方式

  • 全员参与代码评审

  • 每周花费0.5~1天开展CR

  • 严格且彻底的评审

如何拆分 CL

https://google.github.io/eng-practices/review/developer/small-cls.html

Commit 描述

Bad Case:
image.png
“修复错误”是不充分的 CL 描述。什么 bug?你做了什么来修复它?

其他类似的不良描述包括:

  • 逻辑修复

  • 添加补丁

  • 增加XX规则

  • 删除XX逻辑

Good Case:
◆ 摘要:【xx模块】新增xx功能
◆ 背景: 新功能需求,要求xxx, 详见[卡片链接]
◆ 说明: 由于xx,新增xx处理模块…

  • 摘要:删除 RPC 服务器消息自由列表的大小限制

  • 说明:像 FizzBuzz 这样的服务器有非常大的消息,可以从重用中受益。使自由列表更大,并添加一个 goroutine 随着时间的推移慢慢释放自由列表条目,以便空闲服务器最终释放所有自由列表条目。

必要时,应使用 cz 等工具进行规范。

心态

  • 一次 CR 其实是开启的一次“对话”

  • 应该期待评论和反馈,并及时进行回复

  • 做好心理准备自己的代码可能会有缺陷

  • CR 的目的之一就是发现问题, 所以不要有抵触

CR内容

代码是写给人看的, 不是写给机器看的,只是顺便计算机可以执行而已。------《计算机程序的构造和解释》

应该被 CR 的内容:

自上而下,优先级从高到低:

模块简介
设计 代码是否设计良好并适合您的系统?
功能 代码的行为是否符合作者的预期?代码的行为方式对其用户有好处?
安全性 代码是否安全合规?
复杂性 代码可以更简单吗?当其他开发人员将来遇到此代码时,他们是否能够轻松理解和使用此代码?
测试 代码是否具有正确且设计良好的自动化测试?
命名 开发人员是否为变量、类、方法等选择了明确的名称?
注释 注释是否清晰有用?
风格 代码是否遵循京东代码风格规范?
文档 开发人员是否更新了相关文档?

https://google.github.io/eng-practices/review/reviewer/looking-for.html

CR流程顺序

image.png
https://google.github.io/eng-practices/review/reviewer/navigate

京东实际代码片段评审讲解

设计

应该有合理的职责划分,合理的封装

good case

componentDidMount() {
  this.fetchUserInfo();
  this.fetchCommonInfo();
  this.fetchBankDesc();
}

bad case

componentDidMount() {
  const { location, dispatch, accountInfoList } = this.props;
  const { token, af } = getLocationParams(location) || {};
  dispatch({
    type: 'zpmUserCenter/fetchUserInfo',
    payload: {
      token,
    },
  }).catch(e => {
    const zpmOpenAuthUrlLogin = decodeURIComponent(getCookie('zpmOpenAuthUrlLogin'));
    // 如果token过期则跳转回第三方平台
    if ([TOKEN_EXPIRED_CODE, '300000'].includes(e.errorCode) && (isSaveUrl(af) || isSaveUrl(zpmOpenAuthUrlLogin))) {
      setTimeout(() => {
        window.location.href = isSaveUrl(af) ? af : zpmOpenAuthUrlLogin;
      }, 2000);
    }
  });

  if (!this.showWhichHeader() && !this.showGatewayHeader()) {
    dispatch({
      type: 'zpmUserCenter/fetchAccountInfo',
      payload: {
        token,
      },
    });
  }
  this.getBlackList()
}

问题1,fetchUserInfo 未进行封装
问题2,af 命名过于随意
问题3,‘300000’ 魔法字符串
问题4,选择使用 af 或 zpm 这两个URL的逻辑建议封装,避免多次调用 isSaveUrl

优秀代码设计的特质 CLEAN

• Cohesive:内聚的代码更容易理解和查找bug
• Loosely Coupled:松耦合的代码让实体之间的副作用更少,更容易测试、复用、扩展
• Encapsulated:封装良好的代码有助于管理复杂度,也更容易修改
• Assertive:自主的代码其行为和其所依赖的数据放在一起,不与其它代码互相干预(Tell but not Ask)
• Nonredundant:无冗余的代码意味着可以只在一个地方修复bug和进行更改

应避免过度设计

别人在阅读代码时,能清晰辨别我在代码中的设计模式,并且能够随着这个模式继续维护

功能

逻辑正确,逻辑合理,避免晦涩难懂的逻辑

bad case:一段表单代码(原代码过长,仅贴出其中典型的一段)

{ hasQuota ? (
  ['11', '12'].indexOf(invoiceType) === -1 ? (
    <div className="m-b-4 row">
      <div className="col-11">
        <FormItem
          label="基础核验"
        >
    
          {basicVerifyStatus ? '已通过' : <div>
            未通过
            <Tooltip title={basicVerifyMsg} placement="bottom">
              <span className='iconfont icon-tishi theme-primary m-l-1 font-14' />
            </Tooltip>
          </div>}
        </FormItem>
      </div>
      <div className="col-11">
        <FormItem
          label="剩余额度"
        >
          {formatAmount(availableLimit)}
        </FormItem>
      </div>
    </div>) :
    <div className="m-b-4 row">
      <div className="col-11">
        <FormItem
          label="基础核验"
        >
      
          {basicVerifyStatus ? '已通过' : <div>
            未通过
            <Tooltip title={basicVerifyMsg} placement="bottom">
              <span className='iconfont icon-tishi theme-primary m-l-1 font-14' />
            </Tooltip>
          </div>}
        </FormItem>
      </div>
    </div>) :  
  ['11', '12'].indexOf(invoiceType) === -1 ? <div className="m-b-4 row">
    <div className="col-11">
      <FormItem
        label="基础核验"
      >
    
        {basicVerifyStatus ? '已通过' : <div>
          未通过
          <Tooltip title={basicVerifyMsg} placement="bottom">
            <span className='iconfont icon-tishi theme-primary m-l-1 font-14' />
          </Tooltip>
        </div>}
      </FormItem>
    </div>
  </div> :
    null}

关键问题:连续三元判断 + 嵌套三元判断
其他问题:

  • 魔法字符串, 且重复出现

['11', '12'].indexOf(invoiceType) === -1
  • 无意义的空行,严重影响代码阅读

  • FormItem 重复过多

Reviewer 建议:

  1. 对重复代码,梳理内容,进行合理命名

const isNotOnlineInvoice = ['11', '12'].indexOf(invoiceType) === -1;
  1. 每个 FormItem 也进行命名,三元逻辑梳理,重构

 isNotOnlineInvoice trueisNotOnlineInvoice false
hasQuota true verifyCodeFormItem
invoiceCodeFormItem
goodNameFormItem
modifiedDateFormItem
basicVerifyFormItem
availableLimitFormItem
availableLimitFormItem
modifiedDateFormItem
basicVerifyFormItem
hasQuota false verifyCodeFormItem
invoiceCodeFormItem
goodNameFormItem
modifiedDateFormItem
basicVerifyFormItem
basicVerifyFormItem
modifiedDateFormItem

安全性

代码中应注意,不要存储敏感内容

// 微信服务号 生产配置中复写
const WX_APP_ID = 'xxxxxxxxxx';
const WX_APP_SECRET = 'xxxxxxxxxxxxxxxxxxxxx';

// 票将军网站应用配置 (测试环境)
const PJJ_APP_ID = 'xxxxxxxxxx';
const PJJ_APP_SECRET = 'xxxxxxxxxxxxxxxxxxxxx';

一致性

  • 代码一致性:

    • 函数名和实现一致

    • 注释和代码一致

  • 命名方式一致

  • 异步写法一致(promise, async await,callback混用)

  • 抽象层级一致

  • 不建议混用 import 和 require

注释与代码不一致

getCheckboxProps: record => ({
  disabled: !record.basicVerifyStatus || (hasQuota && record.availableLimit <= 0), // 状态为验证成功的时候才可选择使用
})

命名不一致

this.state = {
    isOffline: false, 
    shouldShowFollowLink: false, 
    shouldShowToast: false, 
    ifReceiveNotify: false, 
    bShowAllDocsRedPoint: false,
    isNewPushNotify: false,
}

没事别写注释

good case:
为什么接下来的代码要这么做
bad case:
接下来的代码做了什么

复杂度

  • 优先使用标准库中的能力

  • 封装细节

  • 写的代码越简单, bug越少

  • 尽量遵守单一职责原则

  • DRY——Don’t Repeat Yourself

无意义的函数封装

// 根据admitStatus判断按钮试算前置灰状态
export const isDisabledByAdmitStatus = discountListItem => {
  if (!discountListItem?.bankInfo?.admitStatus) {
    return true
  } else {
    return false
  }
}

建议使用moment、dayjs等标准时间库处理时间:

// 本季度开始时间、结束时间,返回毫秒值
export const getQuarterStartAndEndTime = ({
  time = null,
  isTimestamp = true,
  split = '/',
  startDateTime = ' 00:00:00',
  endDateTime = ' 23:59:59',
} = {}) => {
  let date = checkDate(time) ? new Date(time) : new Date()
  let year = date.getFullYear()
  let month = date.getMonth() + 1
  let startTime = null
  let endTime = null
  if (month <= 3) {
    startTime = year + split + '01' + split + '01' + startDateTime
    endTime = year + split + '03' + split + '31' + endDateTime
  } else if (month > 3 && month <= 6) {
    startTime = year + split + '04' + split + '01' + startDateTime
    endTime = year + split + '06' + split + '30' + endDateTime
  } else if (month > 6 && month <= 9) {
    startTime = year + split + '07' + split + '01' + startDateTime
    endTime = year + split + '09' + split + '30' + endDateTime
  } else {
    startTime = year + split + '10' + split + '01' + startDateTime
    endTime = year + split + '12' + split + '31' + endDateTime
  }

  // 本季度开始时间
  startTime = isTimestamp ? getTimestamp(startTime) : startTime

  // 本季度结束时间
  endTime = isTimestamp ? getTimestamp(endTime) : endTime

  return {
    startTime,
    endTime,
  }
}

DRY——Don’t Repeat Yourself

下面三个方法中重复逻辑非常多,应该进行合理的封装,降低复杂性。另一个比较常见的问题,console.log 这种调试代码不应该被合入主干。

handleMergedInvoice = selectedRows => {
  const { invoiceList } = this.state;
  const { limitNum } = this.props;

  const newInvoiceList = [];
  for (const item of selectedRows) {
    if (newInvoiceList.length && newInvoiceList.length >= limitNum) {
      Message.error(`上传失败,发票数量不可超过${limitNum}张`);
      return;
    }
    item.invoiceUrl = item.invoiceUrl || item.dataUrl || "";
    delete item.dataUrl;
    item.invoiceDate = item.invoiceDate
      ? moment(item.invoiceDate).format("YYYY-MM-DD")
      : "";
    newInvoiceList.push({ ...item, id: getIncrementCid() });
    this.setState({ invoiceList: newInvoiceList }, () => {
      const { invoiceList: list, errIndexList } = this.state;
      if (list.length) {
        this.verifyInvoiceList(list);
      }
      this.invoiceListReduce();
    });
  }
};
updateInvoice = ({ ...data }, i) => {
  const { invoiceList } = this.state;
  const oldInvoice = invoiceList[i];
  data.invoiceUrl = data.invoiceUrl || data.dataUrl || "";
  delete data.dataUrl;
  data.invoiceDate = data.invoiceDate
    ? moment(data.invoiceDate).format("YYYY-MM-DD")
    : "";
  this.setState(
    {
      invoiceList: [
        ...invoiceList.slice(0, i),
        { ...oldInvoice, ...data },
        ...invoiceList.slice(i + 1)
      ]
    },
    () => {
      this.invoiceListReduce();
    }
  );
};
addInvoice = ({ ...data }) => {
  const { invoiceList } = this.state;
  const { limitNum } = this.props;
  if (invoiceList.length && invoiceList.length >= limitNum) {
    Message.error("上传失败,发票数量不可超过500张");
    return;
  }
  data.invoiceUrl = data.invoiceUrl || data.dataUrl || "";
  delete data.dataUrl;
  data.invoiceDate = data.invoiceDate
    ? moment(data.invoiceDate).format("YYYY-MM-DD")
    : "";

  this.setState(
    {
      invoiceList: [...invoiceList, { ...data, id: getIncrementCid() }]
    },
    () => {
      const { invoiceList: list } = this.state;
      if (list.length) {
        this.verifyInvoiceList(list);
      }
      this.invoiceListReduce();
    }
  );
};

封装细节

看到下面这段代码,大概能够想象 newValidate 出现的原因,为了文章阅读体验, 删除部分代码
这个验证函数,严重违反了单一职责,首先包含了多种校验逻辑,还承担了 submit 数据预处理、submit、error处理;不仅如此,还和视图层耦合,包括了回到顶部、定位到错误位置、错误DOM样式调整的逻辑。
当然了,看到newValidate代码行数,也没有好到哪去。
此处200多行代码就成了这个工程的毒瘤。

image.png

validate = () => {
  const { form, totalDraftAmount, output, outputBasename } = this.props;
  const { validateFields } = form;
  const { financeChannel, orderNo: oldOrderNo, checked } = this.state;
  const ocrDomList = document.querySelectorAll('.ocrform');
  const checkboxDomList = document.querySelectorAll('.ant-checkbox');
  // 每次 validate 前先去除上次打的标, Object.values(ocrDomList)为了兼容 ie
  Object.values(ocrDomList).forEach(item => {
    item.classList.remove('field', 'error');
  });
  validateFields(async (err, data) => {
    if (err) {
      Message.warn('请核对填写的内容');
      if (output) {
        // 回到顶部
        scrollToTop();
        return;
      }
      // 定位到第一个错误
      setTimeout(this.goToError(document.querySelector('.field.error')));
      return;
    }
    // 验证发票
    const { contractAmt, draftInfos = {}, invoiceInfos } = data;
    /* 此处省略20行 */
    if (totalDraftAmount > contractAmt) {
      /* 此处省略7行 */
    }
    // 访问子组件中的勾选状态 如没勾选,则校验不通过
    const { checkedA, checkedB } = this.myRef.current.state;
    // 只要有一个没勾选就进来
    if (!checked || !checkedA || !checkedB) {
      // 如果是 确认背书未勾选则 提示相应文案
      Message.warn('请阅读并同意相关服务协议');
      if (output) {
        // 回到顶部
        scrollToTop();
        return;
      }
      // 先打标记,再定位错误
      checkboxDomList[0].classList.add('error');
      // 定位到第一个错误
      setTimeout(
        this.goToError(document.querySelector('.ant-checkbox.error'))
      );
      return;
    }
    let selectedDraftInfo = [];
    /* 此处省略 14 行 selectedDraftInfo 数据组装*/
    const sendData = {
      ...data,
      draftInfos: selectedDraftInfo
    };
    const { couponReleaseNo } = getLocationParams(location) || {};
    if (couponReleaseNo) sendData.couponReleaseNo = couponReleaseNo;
    if (oldOrderNo) sendData.orgOrderNo = oldOrderNo;
    // 用户提交时选择为无重复背书, 删除已上传重复背书文件
    const { billReusedStatus } = this.endorseBillRef.current.state;
    if (billReusedStatus === billReusedStatusEnum.noReuse) {
      delete sendData.repeatedEndorseFileUrl;
    }
    try {
      /* 此处省略 19 行 发起接口调用, 成功 + 失败逻辑处理*/
    } catch (error) {
      this.setState({
        loading: false
      });
      let errMessage;
      // 通过 error.message 字符串中 拿到错误模块对应的 invoiceNo
      errMessage = error.message.split('[')[1];
      if (!errMessage) return;
      errMessage = errMessage.split(']')[0];
      // 通过报错信息定位到错误模块索引
      const errorInvoiceIndex = invoiceInfos.findIndex(
        item => item.invoiceNo === errMessage
      );
      // 添加标红样式
      ocrDomList[errorInvoiceIndex].classList.add('field', 'error');
      if (output) {
        // 回到顶部
        scrollToTop();
        return;
      }
      // 定位到发票错误位置
      setTimeout(this.goToError(ocrDomList[errorInvoiceIndex]));
    }
  });
};

认知复杂度与圈复杂度

整体来说正相关, 也有例外:

function getWords(number) {             // +1
  switch (number) {
    case 1:                             // +1
      return "one";
    case 2:                             // +1
      return "a couple";
    case 3:                             // +1
      return "a few";
    default:
      return "lots";
  }
}                                       // 圈复杂度:4

function getWords(number) {             
  switch (number) {                     // +1
    case 1:                   
      return "one";
    case 2:                 
      return "a couple";
    case 3:                     
      return "a few";
    default:
      return "lots";
  }
}                                       // 认知复杂度:1
function sumOfPrimes(max) {             // +1
  let total = 0;
  for (let i = 1; i <= max; i++) {      // +1
    for (let j = 2; j < i; j++) {       // +1
      if (i % j === 0) {                // +1
        continue;
      }
    }
    total += i;
  }
  return total;
}                                       // 圈复杂度:4

function sumOfPrimes(max) {             
  let total = 0;
  for (let i = 1; i <= max; i++) {      // +1
    for (let j = 2; j < i; j++) {       // +2
      if (i % j === 0) {                // +3
        continue;                       // +1
      }
    }
    total += i;
  }
  return total;
}                                       // 认知复杂度:7

复杂度评判标准

  1. 需要添加“黑客代码(hack)”来保证功能的正常运行。

  2. 总是有其他开发者询问代码的某部分是如何工作的。

  3. 总是有其他开发者因为误用了你的代码而导致出现bug。

  4. 即使是有经验的开发者也无法立即读懂某行代码。

  5. 你害怕修改这一部分代码。

  6. 管理层认真考虑雇用一个以上的开发人员来处理一个类或文件。

  7. 很难搞清楚应该如何增加新功能。

  8. 如何在这部分代码中实现某些东西常常会引起开发者之间的争论。

  9. 人们常常对这部分代码做完全没有必要的修改,这通常在代码评审时,或者在变更被合并进入主干分支后才被发现。
    --- 《编程原则》

规范性

这部分内容比较多,更多内容见 Code Review 手册

import 排序的例子

可以看到第一段代码,没有规律,阅读成本高,第1行, 第5行出现了重复引用。
reviewer建议:
使用工具进行格式化,提高可读性
https://github.com/lydell/eslint-plugin-simple-import-sort
https://github.com/import-js/eslint-plugin-import/

import { ref } from 'vue'
import Taro from '@tarojs/taro'
import gwApi from '@/api/index-gateway-js'
import api from '@/api/index-js'
import { onMounted, reactive, watch } from 'vue'
import InputRight from '../components/InputRight.vue'
import { isweapp, getParams } from '@/utils'
import { FACTORY_ADDRESS_ORIGIN } from '@/utils/const.js'
import { sgmReportCustom } from '@/utils/log'
import { genAddressStr } from '@/utils/address'
import Taro from '@tarojs/taro'
import { onMounted, reactive, ref, watch } from 'vue'

import api from '@/api/index.js' 
import gwApi from '@/api/index-gateway-js' 
import { getParams, isWeapp } from '@/utils' 
import { genAddressStr } from '@/utils/address' 
import { FACTORY_ADDRESS_ORIGIN } from '@/utils/const.js' 
import { sgmReportCustom } from '@/utils/log'

import InputRight from '…./components/InputRight.vue'

命名(世上问题千千万,问题命名占一半)

  • 不用宽泛的模块或文件名

  • 没有拼写错误,单复数也应该正确

  • 符合规范:

    • 文件名kebab-case

    • 类名PascalCase

    • 文件作用域内 常量、变量、函数 camelCase

    • private 是否采用下划线,应保持一致

bad case:

// 无意义命名
let array = [1, 2, 3, 4, 5]
let temp = false
import Part1 from './Part1';
import Part2 from './Part2';
import Part3 from './Part3';
import Part4 from './Part4';

// magic number
let point = CGPoint(x: 123, у: 456)

// 硬编码
reportEvent("ImageClickEventId")

其他

连等

// 连等
elm.onload = elm.onreadystatechange = resolveFn

一段重试逻辑

虽然 if 嵌套不多, 但是让人心智负担很重, 无法快速看出 count 值是多少会 false, 代码写的像面试题

if ((data && data.eid) || count++ > 20) {
  if (!data.eid) {
    webLog.custom({
      type: 1,
      code: 'getEidInfo-empty',
      msg: data,
    })
  }
  clearInterval(timer)
  resolve({ data })
}

reviewer 建议:
使用卫语句提前剔除负向逻辑后, 虽然代码更长, 但是更好理解了。

if (!data.eid && count <= 20) {
  count++
  return
}
if (!data.eid) {
  webLog.custom({
    type: 1,
    code: 'getEidInfo-empty',
    msg: data,
  })
}

clearInterval(timer)
resolve({ data })

CR落地-常见挑战

Code Review时看不出问题

参考解法:
组织集体审查讨论,提升大家审查能力,在代码质量上达成共识

代码审查方式对比

分类方式审查方法优点缺点
审查方式 线下异步审查 时间灵活 实时性差
干扰小
易于存档
面对面审查 实时性好 对审查者干扰大
审查人数 一对一审查 安排容易 多人同时线下审查容易出现重复工作
干扰小
团队集体审查 讨论深入,审查细致 人数多时,容易效率低下
审查范围 增量审查 聚焦重点效率高  
全量审查 系统性 工作量大,不能常常进行
专项集中检查  
审查时机 代码入库前门禁检查 对于把关入库代码的质量,效果很好 太过死板的话,会降低代码入库效率
代码入库前的设计时检查 最早发现问题,从而大大降低问题修复成本;  
代码作者抵触情绪小;  
有效的架构讨论工具;  
代码入库后检查 既不阻塞代码入库,又可以对提交的代码进行审查 有问题代码错过检查而上线的风险

担心冲突、害怕出错

比如 A 看出了不少问题,但是发现代码作者非常不耐烦,导致 A 不敢把看到的所有问题都提出来。
参考解法:

冲突发生

  • 解决冲突
    ✅ Leader协助沟通及仲裁
    ✅ 协商达成共识
    ✅ 寻求第三人评估
    ✅ 组内讨论
    ❌置若罔闻
    ❌放任自由

预防冲突

  • 沟通技巧
    尽量疑向、不要太肯定
    ✅如果采用......是否会更合适?
    ❌这里应该......
    ✅是否考虑过......这样的方案?
    ❌......方案肯定更好
    ✅这个地方似乎会影响滚动性能?
    ❌这样写肯定会影响滚动性能

  • 发现问题,尽量提供建议
    ✅......这样会更简洁
    ❌你这代码复杂度太高了
    ✅根据......项目规范,这里应该这样...
    ❌你这代码不符合项目规范

特别注意

不要吝啬称赞

标签:Code,const,Review,return,CR,import,分享,data,代码
From: https://www.cnblogs.com/Jcloud/p/18103500

相关文章

  • H5网页调用APP原生分享菜单 方法:mcloudshare://advert?imgUrl=图标链接&link=分享的链
    要在H5网页中调用APP原生的分享菜单,你可以通过以下步骤实现:创建分享按钮或触发分享的交互元素,例如一个按钮或链接。在按钮的点击事件处理程序中,使用JavaScript生成一个调用APP分享功能的URL。根据你提供的信息,生成的URL格式如下:mcloudshare://advert?imgUrl=图......
  • 虚拟线程知识分享
    1相关概念-操作系统线程(operatingsystemthreads):即硬件设备配备的线程,一般和服务器硬件的核心数量,例如interCPU的大核拥有两个操作系统线程,小核拥有一个操作系统线程。云服务器的线程数一般和服务器配置上的核心数量相同。-平台线程(PaltformThread):java.lang.Th......
  • LeetCodeHot100 动态规划 70. 爬楼梯 118. 杨辉三角 198. 打家劫舍 279. 完全平方
    70.爬楼梯https://leetcode.cn/problems/climbing-stairs/description/?envType=study-plan-v2&envId=top-100-likedpublicintclimbStairs(intn){if(n<=1)returnn;int[]dp=newint[n+1];dp[1]=1;dp[2]=2;......
  • 除自身以外数组的乘积 - LeetCode 热题 16
    大家好!我是曾续缘......
  • 合并区间 - LeetCode 热题 14
    大家好!我是曾续缘......
  • Leetcode 第 126 场双周赛题解
    Leetcode第126场双周赛题解Leetcode第126场双周赛题解题目1:3079.求出加密整数的和思路代码复杂度分析题目2:3080.执行操作标记数组中的元素思路代码复杂度分析题目3:3081.替换字符串中的问号使分数最小思路代码复杂度分析题目4:3082.求出所有子序列的能量和思......
  • Leetcode 第 388 场周赛题解
    Leetcode第388场周赛题解Leetcode第388场周赛题解题目1:3074.重新分装苹果思路代码复杂度分析题目2:3075.幸福值最大化的选择方案思路代码复杂度分析题目3:3076.数组中的最短非公共子字符串思路代码复杂度分析题目4:3077.K个不相交子数组的最大能量值思路代码......
  • 懒人方法|(一)分享:NHANES数据库怎么下载整理
    1.前言继前面孟德尔随机化的代码分享,应粉丝要求出一篇关于NHANES数据库的数据整理入门教程前面MR代码:全代码分享|R语言孟德尔随机化怎么做?TwoSampleMR包MR一套标准流程2.数据库界面NHANES(NationalHealthandNutritionExaminationSurvey,全国健康和营养调查)是美国进行......
  • 免费分享一套SpringBoot+Vue个人健康管理系统,帅呆了~~
    大家好,我是java1234_小锋老师,看到一个不错的SpringBoot+Vue个人健康管理系统,分享下哈。项目视频演示【免费】SpringBoot+Vue个人健康管理系统Java毕业设计_哔哩哔哩_bilibili【免费】SpringBoot+Vue个人健康管理系统Java毕业设计项目来自互联网,免费开源分享,严禁商业。更多......
  • 腾讯云Ubuntu远程接入Vscode并设置root免密码登录
    最近在尝试Linux编程,想起自己还有一个腾讯云的服务器,就重装了Ubuntu,然后装了环境之后尝试用Vscode连接,但是发现用root用户无论如何都登录不上,后来把用户名换成ubuntu之后就能登录上了,但是在VsCode上写代码时又出现了很多问题。1、某些文件夹打不开,后来发现是用户权限不够,只能去腾......