六则糟糕代码的优化方案

这里分享代码 review 中发现的六则糟糕代码的案例，并进行分析：

案例一

---

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

某前端同学的 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

}