代码评审歪诗

贾言

架构师说, 用20个字描述代码评审的内容， 自省也省人。由于是一字一含义， 不连贯， 为了增强趣味性， 每句都增加对应的歪解。只是对常见评审的描述， 不尽之处，欢迎补充！

  验幻空越重  -- 言欢空月虫

  验：公共方法都要做参数的校验，  参数校验不通过明确抛出异常或对应响应码。

1.   java bean验证已经是一个很古老的技术了， 会避免我们很多问题， 可参考：     

   http://beanvalidation.org/ http://www.infoq.com/cn/news/2010/03/javaee6-validation https://www.sitepoint.com/using-java-bean-validation-method-parameters-return-values/      

2. 在接口中也明确使用验证注解修饰参数和返回值， 作为一种协议要求调用方按验证注解约束传参， 返回值验证注解约束提供方按注解要求返回参数  

  幻：在代码中要杜绝幻数，幻数可定义为枚举或常量以增强其可读性

  空：要时刻警惕空指针异常

1.    常见的 a.equals(b) 要把常量放到左侧  

2.  aInteger == 10 如果 aInteger 为空时会抛出空指针异常  

3.  不确认返回集合是否可为空时要做非空判断， 再做for循环  

4.    使用空对象模式， 约定返回空集合， 而非null  

5.   使用StringUtils判断字符串非空    

  越：如果方法传入数组下标作为参数，要在一开始就做下标越界的校验，避免下标越界异常

  重：不要写重复代码，重复代码要使用重构工具提取重构

  命循频异长 -明勋品宜昌

  命：包/类/方法/字段/变量/常量的命名要遵循规范，要名副其实，这不但可以增加可读性，还可以在起名的过程中引导我们思考方法/变量/类的职责是否合适

有意义很重要， 典型无意义命名：

 public static final Integer CODE_39120 = 39120; public static final String MESSAGE_39120 = "[包裹]与[库房号]不一致，确定装箱？"; public static final Integer CODE_39121 = 39121; public static final String MESSAGE_39121 = "[包裹]与[箱号]的承运类型不一致,确定装箱？";   Rule rule1 = request.getRuleMap().get("1050");

CODE_39120这个名字和幻数没多大区别。

  循：不要在循环中调用服务，不要在循环中做数据库等跨网络操作

  频：写每一个方法时都要知道这个方法的调用频率，一天多少，一分多少，一秒多少，峰值可能达到多少，调用频率高的一定要考虑性能指标，考虑是否会打垮数据库，是否会击穿缓存

  异：异常处理是程序员最基本的素质，不要处处捕获异常，对于捕获了只写日志，没有任何处理的catch要问一问自己，这样吃掉异常，是否合理

下面是一个反例， 在导出文件的controller方法中做了两层的try...catch， 在catch块中记录日志后什么都没做， 这样用户看不到真正想要的内容， 研发也只有看日志才能发现错误， 而“看日志”， 通常只有业务方反馈问题时才会看， 就会导致研发人员发现错误会比现场人员还会晚。

 @RequestMapping(value = "/export") public void export(CityRelationDomain condition, HttpServletResponse response) {    ZipOutputStream zos = null;    BufferedWriter bufferedWriter = null;    try {       condition.setStart(0);       condition.setSize(MAX_EXPORT_LINES);       List list = cityRelationService.getOrdersByCondition(condition);       response.setCharacterEncoding("GBK");       response.setContentType("multipart/form-data");       response.setHeader("Content-Disposition", "attachment;fileName=export.zip");       zos = new ZipOutputStream(response.getOutputStream());       bufferedWriter = new BufferedWriter(new OutputStreamWriter(zos, "GBK"));       bufferedWriter.write("订单类型编码,始发城市-省,始发城市-市,目的城市-省,目的城市-市");       ZipEntry zipEntry = new ZipEntry("export.csv");       zos.putNextEntry(zipEntry);       for (CityRelationDomain domain : list) {          try {             bufferedWriter.newLine();             bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getOrderCode()));             bufferedWriter.write(',');             bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getProvinceNameFrom()));             bufferedWriter.write(',');             bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getCityNameFrom()));             bufferedWriter.write(',');             bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getProvinceNameTo()));             bufferedWriter.write(',');             bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getCityNameTo()));          } catch (Exception e) {             e.printStackTrace();          }       }       bufferedWriter.newLine();            bufferedWriter.flush();            zos.closeEntry();            bufferedWriter.close();    } catch (Exception e) {       e.printStackTrace();       logger.error("导出CSV文件异常");    } finally {       try {          if (zos != null) {             zos.close();          }          if (bufferedWriter != null) {             bufferedWriter.close();          }       } catch (IOException e) {          e.printStackTrace();       }    } }

  长：如果一行代码过长，要分解开来；如果一个方法过长，要重构方法；如果一个类过长要考虑拆分类

  依轮线日简 -依伦先日贱

  依：如果调用了外部依赖，一定要搞清楚这个外部依赖可以提供的性能指标，最好约定SLA

  轮：不要重复造轮子，如果已经有成熟类库实现了类似功能，要优先使用成熟类库的方法，这是因为成熟类库中的方法都经过很多人的测试验证，通常情况下我们自己实现的质量最大等同于成熟类库的质量。

  线：要注意我们的jsf服务， web应用，消费消息的worker都是多线程环境，要注意线程安全问题，最典型的HashMap， SimpleDateFormat， ArrayList是非线程安全的，另外如果使用Spring自动扫描服务，那么这个服务默认是单例，其内部成员是多个线程共享的，如果直接用成员变量是有线程不安全的。

两个典型的错误代码片段：

  1）无视SimpleDateFormat非线程安全

 @Service public class AService {     private static final SimpleDateFormat FORMAT = new SimpleDateFormat("yyyy-MM-dd");     public void doSomething() {         //use FORMAT     } }

  2）使用Service成员变量

 @Service public class BService {     private Pojo b;     public void doB() {          b = getB();          process(b);     } }

  日：打印日志和设定合理的日志级别，如有必要要添加if条件限定是否打印日志，在日志中使用JSON序列化，生成长字符串的toString()都要做if限定打印，否则配置的日志级别没达到，也会做大量字符串拼接，占用很多gc年轻代内存.  另外一定要通过log4j打印日志而不是直接把日志打印到控制台。

典型错误示例：

 @Service public class FooService {     private static final Logger LOGGER = LoggerFactory.getLogger(FooService.class);     public void doFooThing(Foo foo) {         LOGGER.debug("get parameter foo {}", JSONObject.toString(foo));         try {/*do something*/} catch (Exception ex) {ex.printStackTrace();}     } }

  简： 尽可能保持整体设计的简洁， 方法实现的简洁， 要根据情况使用内存缓存， redis 缓存， jmq 异步处理。 这里的简需要把握好分寸。

  接偶正分壮 - 洁偶正粉妆

  接：接口是用来隔离变化的，如果一个业务有几种不同的形态，但都有相同的处理，那么可以定义接口来隔离业务形态的不同，在服务调用处，通过业务类型字段来获得不同的服务类。  而不要实现一个类，然后在类的各个方法中都根据业务类型做if else或更复杂的各种判断。

典型示例：

  做法1：

public interface BarService {    void doBarThing(Bar b);          void doBarFatherThing(Bar b); } public class BarServiceImpl  implement BarService{     public void doBarThing(Bar b) {         if (b.getType() == BarType.A) {             //do some logic         } else (b.getType() == BarType.B) {             //do some B type logic         }         //do other doBarThing logic     }          public void doBarFatherThing(Bar b) {         if (b.getType() == BarType.A) {             //do some logic         } else (b.getType() == BarType.B) {             //do some B type logic         }         //do other doBarFatherThing logic     } }

做法 2 ：

public interface BarService {     void doBarThing(Bar b);          void doBarFatherThing(Bar b); } public class BarServiceFactory {     public BarService getBarService(BarType type) {         // get bar service logic     } } //如果有公共逻辑就定义， 没有就不定义 public class BaseBarService implement BarService {     public void doBarThing(Bar b) {         //do other doBarThing logic     }          public void doBarFatherThing(Bar b) {         //do other doBarFatherThing logic     }      } public class TypeABarService extends BaseBarService  implement BarService {     public void doBarThing(Bar b) {         // doATypeThing         super.doBarThing(b);     }          public void doBarFatherThing(Bar b) {         // do bar type A service super.doBarFatherThing(b); //如果需要就调用， 不需要就不调用父类     }      }

  做法2的好处是将不同类型的逻辑解耦，各自发展，不会相互影响，如果添加类型也不必影响现有类型逻辑。

  偶：认识系统之间的耦合关系，通过同步数据来做两个系统之间的交互是一种很强的耦合关系，会使数据接收方依赖于数据发送方的数据库定义，如果发送方想改数据结构，必须要求下游接收方一起修改；通过接口调用是一种常见的系统耦合关系，接口的提供方要保证接口的可用性，接口的调用方要考虑接口不可用时的应对方案； mq消息是一种解耦的方法，两个系统不存在实时的耦合关系。但是mq解耦的方式不能滥用，在同一系统内不宜过多使用mq消息来做异步，要尽可能保证接口的性

  能，而不是通过mq防止出问题后重新消费。

  正：模块之间依赖关系要正向依赖，不能让底层模块依赖于上层模块；不能让数据层依赖于服务层也不能让服务层依赖于UI层；也不能在模块之间形成循环依赖关系。

  分：分而治之，复杂的问题要分解成几个相对简单的问题来解决，首先要分析出核心问题，然后分析出核心的入参是什么，结果是什么，入参通过几步变化可以得出结果。

  壮：时刻注意程序的健壮性，从两个方面实践提升健壮性：

  1）契约，在设计接口时定义好协议参数，并在实现时第一时间校验参数，如果参数有问题，直接返回给调用方； 如果出现异常情况， 也按异常情况约定应对策略

2）  考虑各种边界条件的输出， 比如运单号查询服务， 要考虑用户输入错误运单时怎么返回， 有边界的查询条件， 如果用户查询条件超过边界了， 应该返回什么

  3   ）为失败做设计，如果出问题了有降级应对方案。

原文链接：http://outofmemory.cn/wiki/sa-said-code-review