浅谈 code review

573 阅读6分钟

前言:之前看过一篇文章,大概的意思是code review的核心不是为了发现bug,而是提出 “这样做技术上没有错,但思路和扩展性上存在问题”这类改进意见。 相信大家做过不少code review,当代码提交后,被一个个加上comments打回后很不好受,但其实在这个过程中就学习到了代码规范和很多好的实践。

一.什么是code reivew?

其实就是代码再次查看评审

二.为什么要做code Review?

1.提高代码质量

    Code reviews 不应该承担发现代码错误的职责。Code Review主要是审核代码的质量,如可读性,可维护性,以及程序的逻辑和对需求和设计的实现。

2.形成团队统一风格

    Code reviews 不应该成为保证代码风格和编码标准的手段。编码风格和代码规范都属于死的东西,每个程序员在把自己的代码提交团队Review的时候,代码就应该是符合规范的,这是默认值,属于每个人自己的事情,不应该交由团队来完成,否则只会浪费大家本来就不够的时间。

3.个人快速成长

    通过有效的代码 Review,Author 和 Reviewer 都将获得成长,在 Review 过程中参与人就具体的问题展开深入的讨论,无论是怎么写出整洁的代码、怎么更好地更全面地思考问题、怎么最佳地解决问题,参与人都可以互相取长补短,互相提高。通过具体代码实现进行的讨论往往是最深入和有效的,代码 Review 是开发者提高代码能力最重要的途径之一。

三.代码举例

1.命名更合理

案例一(常量命名)

// bad
const url= 'http://site.com/graphql';

// good
const GLOBAL_URI = 'http://site.com/graphql';

案例二(变量命名)

// bad
const a = 1;
const _obj = {};

// good
const age = 1;
const user = {};

案例三(组件命名)

// bad
vue.component('todo',{
    //...
})
export default{
    name: 'Todo'
}

// good
vue.component('todo-item',{
    //...
})
export default{
    name: 'TodoItem'
}

案例四(函数命名)

// bad
export default{
    methods(){
        getList(){
        //
        }    
    }
}

// good
export default{
    methods(){
        getGoodsList(){
            //
        }    
    }
}

// good
export default{
    methods(){
        getGoodsListToRefresh(){
            //
        }    
    }
}

2.代码更简洁

案例一

// bad
export default{
    methods(){
        isFlag(){
            let flag = false;
            if(arr.filter(item=>item.isEdit).length){
                flag = true;
            }
            return flag;
        }    
    }
}

// good
export default{
    methods(){
        isFlag(){
            return arr.some(item=>item.isEdit)
        }
    }
}

案例二

// bad
let conditions=[ 'str1', 'str2' ];
isFlag(str){
    if(str.includes("str1") || str.includes("str2")){
        return true;
    }else{
        return false;
    }
}


// good 
isFlag(str){
    let conditions=[ 'str1', 'str2' ];
    return conditions.some(condition=>str.includes(condition))
}

案例三

// bad
<script>
export default {
    data(){
        model1:false,        
        model2:false,
        model3:false,
    }
}

</script>

// good
<script>
export default{
    data{
        modelType:'' // 类型为 model1,model2.model3
    }
}
</script>

案例四

// bad
let nickName=nickName?nickName:'果宝宝';

// good
let nickName = nickName|| '果宝宝'

案例五(慎用三元表达式的嵌套)

// bad
let message = locked===1? '申请提现':(locked===2?'账户冻结中' :'账户不存在')

// good
let meaasgeList = {
    1:'申请提现',
    2:'账户冻结中',
}
let message = meaasgeList[locked]||'账户不存在';

案例六(避免过多的函数参数)

// bad 
function person(name,age,like){
    return `name is ${name},age is ${age},like ${like}`;
}
console.log(person('张三','24', 'play games'));

//good
function person({name,age,like}){    
    return `name is ${name},age is ${age},like ${like}`;
}
const obj={
    name:'张三',
    age:'24',
    like:'play games'
}
console.log(person(obj));

案例七(使用Object.assign 设置默认对象)

// bad
const obj= {
    name:'name',
    age:'age',
    like:'like'
}
function person(obj){
    obj.name = obj.name|| '张三';
    obj.age = obj.age|| '24';
    obj.like = obj.like|| 'play games';
    return obj;
}
console.log(person(obj))

// good
const obj= {
    name:'name',
    age:'age',
    like:'like'
}
function person(obj){
    const newPerson = Object.assign({
        name:'张三',
        age:'24',
        like:'play games'
    },obj);
    return newPerson;
}
console.log(person(obj))

案例八

// bad
function(isGift){
    if (isGift === 'Y') {
      this.setData({
        btnText: '抱歉,赠品不支持购买',
        sellOut: true
      });
    } else if (isGift === 'D') {
      this.setData({
        btnText: '购买已达上限',
        sellOut: true
      });
    } else if (isGift === 'M') {
      this.setData({
        btnText: '已下架',
        sellOut: true
      });
    }else {
      this.setData({
        sellOut: false
      })
    }
}

// good
const typeList = {
    'Y':'抱歉,赠品不支持购买',
    'D':'购买已达上限',
    'M':'已下架'
}
function(isGift){
    this.setData({
        sellOut: false
    })
    if(['Y', 'D', 'M'].includes(isGift)){
        setMessage(isGift)
    }
}

function setMessage(type){
    this.setData({ btnText: typeList[type], sellOut: true });
}

案例九(console.log打印删除)

经常会出现大量的console.log打印带到生产环境,虽然有工具可以在构建删除,但是建议开发者不要过度依赖console来做调试,而且用完及时删除保持代码整洁,养成用完console之后及时删除的习惯。

3.性能更优

案例一(将大量和渲染无关的数据直接放置 在data中,将会影响性能)

// bad
<script>
export default{
    data(){
        list:{
            test1:'测试1',
            test2:'测试2',
            test3:'测试3'
        }
    }
}
</script>

// good
<script>
const LISTDATA = {            
    test1:'测试1',
    test2:'测试2',
    test3:'测试3'
}
export default{
    data(){
        list:Object.freeze(LISTDATA)
    }
}
</script>

案例二(同一组的v-if 和 v-else 最好使用key标识)

<div v-if='error' key='search-status'>
    错误{{error}}
</div>
<div v-else key='search-results'>
    {{results}}
</div>

案例三( 样式尽量不要用标签,提高CSS匹配性能)

// bad
<template>
    <view class="box">
        <view></view>
    </view>
</template>
<style scoped lang='scss'>
    .box{
        view{
            font-size:24rpx;
        }
    }
</style>

// good 
<template>
    <view class="box">
        <view class="content"></view>
    </view>
</template>
<style scoped lang='scss'>
    .box{
        .content{
            font-size:24rpx;
        }
    }
</style>

案例四(vue中的this滥用)

// bad
methods: {
  getData () {
      const params = {
          page: this.state.page,
          pageSize:this.state.pageSize
      }
      axios.get(url).then(res => {
          if (res.length > this.state.total) {
              this.noMore = true;
          }
      })
  }  
}

// good
// 利用解构获取值,不用每次从this访问
methods: {
  getData () {
      const { page, pageSize, total } = this.state;
      const params = {
          page,
          pageSize
      }
      axios.get(url).then(res => {
          if (res.length > total) {
              this.noMore = true;
          }
      })
  }  
}

4.可读性更强

案例一(类型转换)

// bad
const isOpen = !!data.length;
const num = "111" * 1;
const str = 111 + "";

// good
const isOpen = Boolean(data.length);
const num = Number("111");
const str = String(111);

案例二(代码逻辑分离)

// bad
function getList () {
    axios.get(url).then(res => {
        // 处理数据格式
        ...
        
        // 处理事件A
        ...
        
       // 处理事件B
       ...
       
       // 处理事件C
       ...
    })
}

// good
function getList () {
    axios.get(url).then(res => {
        // 处理数据格式
        hanldeData();
        
        // 处理事件A
        handleA();
        
       // 处理事件B
       handleB();
       
       // 处理事件C
       handleC();
    })
}

案例三(缓存处理)

// bad
// pageA
function updateUser () {
    window.sessionStorage.setItem('user', { name: "张三" })
}

// pageB
function updateUser () {
    window.sessionStorage.setItem('user', { name: "李四" })
}
    
// good
// pageA
function updateUser () {
    window.sessionStorage.setItem('pageA:user', { name: "张三" })
}

// pageB
function updateUser () {
    window.sessionStorage.setItem('pageB:user', { name: "李四" })
}

案例四(多场景视图)

// bad
<template>
    <div class="a">
        <div v-if="showB" class="b">
            B
        </div>
        <div v-else class="c">
            C
        </div>
        <div class="view">
            <div v-if="showB">bbbb</div>
            <div v-else>cccc</div>
        </div>
        <div class="view">
            <div v-if="showB">BBBB</div>
            <div v-else>CCCC</div>
        </div>
    </div>
</template>

// good
<template>
    <div class="a">
        <!--BBlock-->
        <template v-if="showB">
            <div class="b">
                B
            </div>
            <div class="view">
                <div>bbbb</div>
            </div>
            <div class="view">
                <div>BBBB</div>
            </div>
        </template>
        
        <!--cBlock-->
        <template v-else>
            <div class="c">
                C
            </div>
            <div class="view">
                <div>cccc</div>
            </div>
            <div class="view">
                <div>CCCC</div>
            </div>
        </template>
        
    </div>
</template>

5.注释更到位

案例一(枚举值)

// bad

const paramsA = {
    sort: 1
}

// good
/*
    @param {number} sort 排序规则
    @property {1} 升序
    @property {2} 倒叙
**/
const paramsA = {
    sort: 1
}

案例二

// bad
// 深拷贝
  deepClone(copyObj) {
    if (typeof copyObj !== 'object' || !copyObj) {
      return copyObj;
    }
    const newObj = JSON.parse(JSON.stringify(copyObj));
    return newObj;
  },

// good
/**
   * @description: 深拷贝
   * @param {object, array} copyObj 需要被拷贝的对象或数组
   * @return {object, array} newObj 深拷贝副本
   */
  deepClone(copyObj) {
    if (typeof copyObj !== 'object' || !copyObj) {
      return copyObj;
    }
    const newObj = JSON.parse(JSON.stringify(copyObj));
    return newObj;
  },

四. git commit提交不规范

在多人协作中,git提交的commit跟代码注释一样重要,很多时候开发并没有注意commit的内容,随意提交导致有时候回退版本时候根本不知道哪个commit是能回退的

// bad

git commit -m "bugfix"
git commit -m "修改样式"
// 以上提交完全看不出是在改什么东西

// good
// 正常来说commit是以功能点或者bug来合并提交的,比如版本A,有a,b,c三个功能点
// 那这个版本的commit合并到主分支的commit应该为3个commit
// 比如a有问题临时拆分版本,需要上线b,c,那直接从主分支cherry-pick b,c就可以了
// 最好用git hook强制并遵循 git commit -m "<type:提交类型>(<scope:影响范围>): <subject:提交内容>"
git commit -m "feat(首页): 增加首页banner"
git commit -m "fix(首页): 顶部用户头像展示错误"
git commit -m "style(首页,我的页): banner大小调整"

五.如何优化自己的代码?

  • 不要写烂代码(烂变量名,烂注释,烂设计)
  • 避免重复
  • 坚持每天优化

六.结合项目

  1. 提交代码前,开发人员的自查非常重要!!!
  2. git comment 提交要规范
  3. 提测之前,首先将dev分支合并到test分支,创建合并请求时添加评审者(参与项目的开发,技术经理等)

image.png

4.评审者可以coding上增加comment,代码没有问题的话可以评论“LGTM”

image.png

5.被comment的部分,开发修改后可以标记

image.png 6.待评审者全部审核通过后,可以合并代码,并提测

7.测试过程中有需要调整的内容,修改进行自测后,重新发起合并请求,进行代码审核

总结

最合适的审核者通常是代码的主人。建议大家都能多参加代码review。