codereview常见问题

研发管理 研发管理, 笔记 评论

路线图

常见代码问题

常见的潜在代码问题是当前直接会导致BUG、故障或者产品功能不能正常工作的类别。

空值

空值是最容易的问题

  • 值为NULL导致空指针异常；
  • 参数字符串含有前导或后缀空格没有Trim导致查询为空。

原则上，对于任何异常， 希望能够打印出具体的错误信息，根据错误信息很快明白是什么原因， 而不是一个 null ，还要在代码里去推敲为什么为空。这样我们必须识别出程序中可能的null， 并及时检测、捕获和抛出异常。

对于空值，最好的防护是“防御式编程”。当获取到对象之后， 使用之前总是判断是否为空，并适当抛出异常、打错误日志或做其它处理。 有的人嫌检测为空的 if 语句充斥在代码里会破坏代码的可维护性， 对此我的建议是：

空值检测一定要有， 有胜于无。
在空值检测总是存在的前提下， 可以优化空值检测的方法和存在形式。 比如集中于一个类 NullChecker 中管理，并与系统的整体错误处理设计保持一致。集中管理和处理一致性原则可以作为系统设计的一个准则。 这样主流程中只要增加一行调用即可， 既可以天网恢恢疏而不漏地检测对象为空， 也不会让代码显得难看。

1
2
3
4
5
class NullChecker {
  public static void checkNull(Object obj, Error error) {
    if (obj == null)  { throw new BizException(error); }
  }
}

在参数入口处统一做 trim。 如果在业务逻辑里做 trim ， 就会导致有的业务逻辑做了 trim ， 有的没做， 体现在产品上就会有令用户困惑的事情发生。 比如搜索和导出业务， 搜索能搜索出来， 导出却没有。

未捕获潜在的异常

第二个容易出错的地方是未捕获潜在的异常。调用API接口、库函数或系统服务等，只顾着享受便利却不做防护，常导致因为局部失败而影响整体的功能。最好的防护依然是“防御式编程”。 要么在当前方法捕获异常并返回合适的空值或空对象，要么抛给高层处理。

切不可默默”吞掉错误和异常”。 如果这样做了， 出问题了等着加班和耗费大量脑细胞吧！
在CodeReview的时候一定要仔细询问：这里是否可能会抛出异常？如果抛异常会怎么处理？是否会影响整体服务和返回结果？

低性能

低性能会导致产品功能不好用、不可用，甚至导致产品失败。

常见情况有：

  1. 循环地逐个调用单个接口获取数据或访问数据库;
  2. 重复创建几乎完全相同的(开销大的)对象；
  3. 数据库访问、网络调用等服务未处理超时的情况；
  4. 多重循环对于大数据量处理的算法性能低；
  5. 大量字符串拼接时使用了String而非StringBuilder.

处理建议

  1. 最好提供批量接口或批量并发获取数据；
  2. 将可复用对象抽离出循环，一次创建多次使用；
  3. 设置合理的超时时间并捕获超时异常处理；
  4. 使用预排序或预处理， 构造合适的数据结构， 使得算法平均性能在 O(n) 或 O(nlogn) ；
  5. 少量字符串拼接使用String, 大量字符串拼接使用 StringBuilder， 通常不会使用到 StringBuffer.

影响范围过大

对多个模块依赖的公共函数的修改，容易造成影响范围超过当前业务改动，无意识地破坏依赖于该公共函数的其他业务。要特别慎重。可靠的方式是：先查看该公共函数的调用， 如果只有自己的业务用，可适当大胆一些； 如果有多个地方依赖，抽离一个新的函数，抽离原函数里的可复用部分，然后基于可复用部分构建新的函数。修改原则遵循“开闭”原则，才能尽可能使改动影响降低到最小化。

基类及实例字段和方法也属于公共函数的范畴。 尽量不要修改基类的东西。

单测问题

单测是保证工程质量的第一道重要防线。单测问题一般包括：

  • 单测未全部通过；
  • 重要业务逻辑缺乏单测；
  • 缺乏异常单测；
  • 代码变更或BUG修复缺乏单测。

单测全部通过应当是提交代码到代码库以及代码Review的前提条件。代码提交者应当保证单测全部通过。没有捷径可走。仅当单测全部通过才提交到代码库， 可以通过工具自动化实现。 对于 maven 管理的工程， 只需一个命令：

1
mvn test && git push origin branch_name 。

单测应当更注重质，而非单纯追求覆盖率。

缺乏单测的重要业务逻辑就像裸露在空气中的电线一样，虽然能跑起来，却是很容易“触电”的。 方法： 增加覆盖比较全面的单测。

缺乏异常单测也是代码提交者常忽略的问题。 异常也是一种实际的业务场景，反映系统的健壮性和友好性。异常应该有相应的单元测试覆盖。创建条件使之抛出异常，并判断异常是否是指定异常；若没有抛出异常或者不是指定异常，则应该 AssertFailed 而不是通过。

对于代码变更和BUG修复，如果当时由于时间紧而没有写，后续应当补上。对于每个代码变更和BUG，都可以抽离出相应的代码部分， 并有相应单测覆盖，并注明原因。

与原有业务逻辑不兼容

改动针对当前需求是合理的，却与原有业务逻辑不兼容，也是常见的问题。比如增加一个搜索条件， 却不能与原有条件联合查询。

与原有业务不兼容， 一般出现在：

一对一与一对多的变化。 比如原来的关系是一个订单对应一个物流信息， 后来变化为一个订单可能对应多个物流信息； 原来的逻辑是一个订单显示多个物流信息可以更改，后来要求一个订单只展示最近一次的物流信息可以修改。
多个业务组合。 业务 A 与业务 B 原来是分开发展的， 后来开展一种活动，将业务A与业务B进行一种组合营销。 此时，多半会出现很多 if-else 语句。

业务逻辑的兼容问题一般体现在系统的复用性和可扩展机制上。良好的系统可复用性和可扩展性可以更容易地做到业务逻辑兼容。 主要有如下几种级别：

自动兼容。 增加一种类型， 只是 biz_type 的值多了一种， 系统自动将已有功能适配给新的 biz_type；
一点改动。增加一个分支语句， 对 biz_type 的某个特性进行扩展；
一些改动。 需要见缝插针地增加一个单独的分支判断和逻辑处理模块， 对整体可扩展性没有影响， 但会造成局部的复杂化；
一部分功能改动。 只需要对其中一个功能模块做个扩展；
多处改动。 需要对多个功能模块做相应的改造，不过更多是新增而不是修改；
难以改动。 需要深入到功能模块内部做艰难的修改， 并要保证原有功能不受影响。

如何应对呢？

针对关联关系， 在项目之初， 可以询问清楚： 将来在产品上是否有可扩展的变化？ 及早预留空间， 或者确定产品上的对策； 在代码实现上， 兼顾考虑一对一到一对多，或一对多到一对一的关联变化。比如使用列表来表达单个信息， 使用索引从列表中获取单个信息。
针对业务组合， 明确各业务的核心部分， 抽离出业务的可复用的部分，形成 API ； 考虑组合模式和装饰器模式来进行扩展。

核心不变， 外围定制化。

缺乏必要日志

对于重要而关键的实例状态、代码路径及API调用，应当添加适当的INFO日志；对于异常，应当捕获并添加Error日志。缺乏日志并不会影响业务功能，但出现问题排查时，就会非常不方便，甚至错失极宝贵的机会(不易重现的情况尤其如此)。此外，缺乏日志也会导致可控性差，难以做数据统计和分析。

错误码不符合规范

错误码本身不算是代码问题，不过基于整个组织和工程的可维护性来说，可以将错误码不符合规范作为一种错误加以避免。方法： 对错误码进行可控的管理和遵循规范使用。可以使用公共文档维护， 也可以开发错误码管理系统来避免相同的错误码。

参数检测缺乏或不足

参数检测是对业务处理的第一层重要过滤。如果参数检测不足够，就会导致脏数据进入服务处理，轻则导致异常，重则插入脏数据到数据库，对后续维护都会造成很多维护成本。方法： 采用“契约式编程”，规定前置条件，并使用单测进行覆盖。

对于复杂的业务应用， 优雅的参数检测处理尤为重要。 根据 “集中管理和处理一致性原则”， 可以建立一个 paramchecker 包， 设计一个可复用的微框架来对应用中所有的参数进行统一集中化检测。参数检测主要包括： (1) 参数的值类型， 可以根据不同值类型做基础的检测； (2) 参数的业务类型， 有基础非业务参数， 基础业务参数和具体业务参数。 不同的参数业务类型有不同的处理。 将参数值类型与参数业务类型结合起来， 结合一致性的异常捕获处理， 就可以实现一个可复用的参数检测框架。参数检测既可以采用普通的分支语句，也可以采用注解方式。采用注解方式更可读，不过单测编写更具技巧。

引用错误

对于动态语言， 由于缺乏强大的静态代码检测，修改了类引用的地方尤其要注意，很可能导致依赖的其他业务出错； 尤其是修改重名引用时。有线上故障教训。PHP工程中含有两个 Format 类， 一个基础的一个业务相关的， 被改动的类文件里开始没有指明引用，默认采用了基础 Format 类的实现， 然后提交者在改动文件头增加了对业务 Format 的引用， 导致依赖于基础Format类的其他业务不能正常工作。避免引用错误的方法： 当要在文件里增加新的类引用时， 先在文件里搜索是否有重名类的引用。如果有， 就要格外小心了。

细节错误

比如数组越界、JSON解析出错、函数参数传递出错、API 版本不对、使用网上拷贝的未经测试的代码、不成熟的算法、传值与传引用、相等性比较等。

对于数组越界错误， 通常要对空数组、针对数组大小的边界值+1和-1写单测来避免； 使用网上拷贝的代码，诚然可节省时间，也一定要加工一下并用单测覆盖； 传值和传引用可通过单测来避免错误； 对象的相等性比较切忌使用等号=。

多重条件

类似 if ((!A || !B) && C || (D && E)) 的多重条件要仔细推敲。方法： 最好拆分成多个有含义变量。 isNotDelay = !A || !B ; isNormal = C ; isAllow = D && E ; cond = isNotDelay && isNormal || isAllow 。

文不符实

文不符实是一种可能导致线上故障的错误。比如一个 getXXX 的函数，结果里面还做了 add, update 的操作。对问题排查、产品运维等都有非常大的杀伤力。因此命名一定要用实质内容相符，除非是故意搞破坏。

跨语言或跨系统交互

稍具规模的互联网创业公司通常会采用多语言开发，比如PHP作为前端，Java作为后台服务。当动态类型语言与静态类型语言交互时，会有一些问题产生。比如PHP的对象通常是一个Map, 如果是空对象就会写成 [], 然而 [] 会被 Java 解析成列表。这样， 如果数据库的值是通过 PHP 写入，那么这个值既有可能是JSON对象字符串，也可能是空数组字符串， Java 来解析就有点尴尬了。 同样，当 Java 调用 PHP 接口时， 不规范的PHP接口既可能返回列表，也可能返回 true or false ， Java 解析返回结果也会比较尴尬。 因此， 在跨语言交互的边界处，要特别注意这些类型转换的差异。

跨系统交互则主要是接口设计与约定的问题。同一个项目里不同业务团队之间的业务接口设计与约定， 不同企业里开放接口的设计与约定， 要在最初深思熟虑，一旦开放，在后期很少有接口设计改动的空间。开放接口设计要符合小而美、正交的特性， 命名要贴切一致， 参数取值要指明约束，枚举参数要给出列表， 结果返回要规范一致，可以采用通用的 {“code”:200, “msg”: “success”, “data”: xxx} 。跨系统交互也要统一对术语和接口的理解的一致。