分析团队中的六则糟糕代码

不念不忘少年蓝@ 2023-01-11 13:11 111阅读 0赞

最近进行代码 review，发现团队中部分新人写的代码比较乱，有一个新人竟然写出了下面的代码：

![format_png][] horrible-code

真是令人哭笑不得，我说你这是在放大招吗？

![format_png 1][] 放大招

这里节选了团队中六则糟糕代码的案例进行分析：

## 案例一 ##

> 变量、属性和函数名应该使用小驼峰式命名法，并且名称是可描述的. 应该避免使用单字符变量和不通用的缩写。

某前端同学的 angular 代码：

export class PageComponent implements OnChanges {
      @Input() pageObj
      @Input() currentPage ? = 0 
    
      goPage
      cg: any = false
    
      goToPage() {
        if (!this.goPage) {
          return
        }
        this.currentPage = +this.goPage
        this.pageChange.emit(this.goPage)
      }
    }

该同学定义了一个变量叫 `cg`，不符合**变量名可描述**的规则，除了本人之外团队其他成员看不懂其含义。`goToPage` 和 `goPage` 容易混淆，语义也不明确。

## 案例二 ##

> 尽量使用 es6 语法简化代码逻辑

某后端同学的 js 代码：

let startDay = 0 
    let endDay = 1  
    switch (query.birth) {
        case  0~1 :  //当天过生日
          startDay = 0
          endDay = 1
          break
        case  1~8 :  //1~8天过生日
          startDay = 1
          endDay = 8
          break
        case  8~16 :  //8~16天过生日
          startDay = 8
          endDay = 16
          break
        case  16~31 :  //16~31天过生日
          startDay = 16
          endDay = 31
          break
        case  31~999 :  //31天以后天过生日
          startDay = 31
          endDay = 999
          break
    }

写了20多行，其实就是一句话能搞定的事情，基本功太差：

let [startDay, endDay] = query.birth.split( ~ ).map(it => +it)

## 案例三 ##

> 使用 `/** ... */` 作为多行注释。包含描述、指定所有参数和返回值的类型和值。
> 
>     /** 
>     * 函数说明 
>     * @关键字 
>     */
> 
> 使用 `//` 作为单行注释。在评论对象上面另起一行使用单行注释。在注释前插入空行。

某前端同学的 angular 代码：

/**处理右上角btn操作**/
    handleWithBtn(btn) {
      switch (btn) {
        case  export : { // 批量认证
          Debug.log( 导出 )
          break
        }
      }
    }

上面的注释既不规范，也是多余的，当起了一个好的名字之后，代码就已经非常明确了。

## 案例四 ##

> 逻辑互斥的 if 语句一定要配合 else 或 return 使用，把概率高的写在前面。

某后端同学的 js 代码：

if (productClass ===  Card  && action === BUYCARD) {
      seneca.sendSms(smsData, params)
    }
    if (productClass ===  Card  && action === TURNCARD) {
      seneca.patchStatus(productId)
    }
    if (productClass ===  Card  && action === REPLACE) {
      seneca.changeStatus( crm ,  Card )
    }
    if (productClass ===  Lesson ) {
      seneca.changeStatus( course ,  Lesson )
    }
    ...

上面每个判断都要执行一次，完全没有必要，这种情况下要么使用 switch 要么 if 配合 else 或 return 使用。

## 案例五 ##

> 保持函数简短，一个好的函数适合展现在一个幻灯片(slide)上，这样如果在一个比较大房间中，也便于最后一排的人阅读。每一个函数的代码应该限制在 15 行左右，另外为了避免 if 语句过度嵌套, 应该提前将函数值返回.

某前端同学为了去除 params 对象中的 value 为 null，  ，undefined 的 key 写的代码：

getUrlParam(sUrl, sKey) {
      const param = sUrl.split( # )[0].split( ? )[1]
      if (param) {
        if (sKey) {       // 指定参数名称
          const strs = param.split( & )
          const arrs = new Array()  // 如果存在多个同名参数，则返回数组
          for (let i = 0, len = strs.length; i < len; i++) {
            const tmp = strs[i].split( = )
            if (tmp[0] === sKey) {
              arrs.push(tmp[1])
            }
          }
          if (arrs.length === 1) {// 返回该参数的值或者空字符串
            return arrs[0]
          } else if (arrs.length === 0) {
            return   
          } else {
            return arrs
          }
        } else {// 不指定参数名称，返回全部的参数对象 或者 {}
          if (param === undefined || param ===   ) {
            return {}
          } else {
            const strs = param.split( & )
            const arrObj = new Object()
            for (let i = 0, len = strs.length; i < len; i++) {
              const tmp = strs[i].split( = )
              if (!(tmp[0] in arrObj)) {
                arrObj[tmp[0]] = []
              }
              arrObj[tmp[0]].push(tmp[1])
            }
            return arrObj
          }
        }
      } else {
        return   
      }
    }

这种函数可维护性极差，自己写的过个星期也读不懂什么意思了，出现错误很难定位。下面是改造后的：

function filterParams(obj) {
      const keys = Object.keys(obj)
      keys.forEach(key => {
        const value = obj[key]
        if (isObject(value)) filterParams(value)
        if (isEmpty(value)) delete obj[key]
      })
      return obj
    }
    
    function isEmpty(input) {
      return [  , undefined, null].includes(input)
    }
    
    function isObject(input) {
      return input !== null && (!Array.isArray(input)) && typeof input ===  object 
    }

明显清晰很多，可读性很强，逻辑也很健壮。如果你觉得一个 15 行以内的函数搞不定某个事情，就把它拆分成多个小于 15 行的函数。

## 案例六 ##

> 配置要写在配置文件里面统一管理，常量也要定义在单独的文件里面，常量名全部大写。

某后端同学写的 js 代码：

let client = new TopClient({
       appkey :  12345678 ,
       appsecret :  asdfasdfasdfasdfasdfasdf ,
       REST_URL :  http://gw.api.taobao.com/router/rest 
    })
     let sign = handler.sign(data,  qwerqwerqwerqwerqwer )
     ...

密钥这种配置信息写在代码里面，既不方便测试，又不利于拓展。

> 作者：thyme
> 
> https://juejin.cn/post/6906366633741287437

[format_png]: /images/20221119/026980427ef842bb92bb821e8848dad6.png
[format_png 1]: /images/20221119/0d678ad36cbe4be5a91a36ed4867e534.png