最佳实践-代码评审歪诗

本文有赵玉开老师总结整理,经本人同意转载至此。如需转载,请务必保留出处!

本文作者:赵玉开,十年以上互联网研发经验,2013年加入京东,在运营研发部任架构师,期间先后主持了物流系统自动化运维平台、青龙数据监控系统和物流开放平台的研发工作,具有丰富的物流系统业务和架构经验。在此之前在和讯网负责股票基金行情系统的研发工作,具备高并发、高可用互联网应用研发经验。


贾言

验幻空越重,

命循频异长。

依轮线日简,

接偶正分壮。

言欢空月虫,

明勋品宜昌。

依伦先日贱,

洁偶正粉妆。


贾言

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

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

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

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

空:   要时刻警惕空指针异常,   常见的 a.equals(b) 要把常量放到左侧, aInteger == 10 如果 aInteger 为空时会抛出空指针异常

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

重:   不要写重复代码,   重复代码要使用重构工具提取重构


命循频异长 -   明勋品宜昌

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

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

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

异:   异常处理是程序员最基本的素质,   不要处处捕获异常,   对于捕获了只写日志,   没有任何处理的 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<CityRelationDomain> 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 年轻代  内存

典型错误示例:

@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));
     }
}

简:   尽可能保持整体设计的简洁,   方法实现的简洁,   要根据情况使用内存缓存, 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)   为失败做设计,   如果出问题了有降级应对方案。





  • 1
    点赞
  • 0
    收藏
    觉得还不错? 一键收藏
  • 0
    评论

“相关推荐”对你有帮助么?

  • 非常没帮助
  • 没帮助
  • 一般
  • 有帮助
  • 非常有帮助
提交
评论
添加红包

请填写红包祝福语或标题

红包个数最小为10个

红包金额最低5元

当前余额3.43前往充值 >
需支付:10.00
成就一亿技术人!
领取后你会自动成为博主和红包主的粉丝 规则
hope_wisdom
发出的红包
实付
使用余额支付
点击重新获取
扫码支付
钱包余额 0

抵扣说明:

1.余额是钱包充值的虚拟货币,按照1:1的比例进行支付金额的抵扣。
2.余额无法直接购买下载,可以购买VIP、付费专栏及课程。

余额充值