代码评审需要注意的项目规范

来源:互联网 发布:php 设置脚本执行时间 编辑:程序博客网 时间:2024/05/17 01:11


贾言

验幻空越重,

命循频异长。

依轮线日简,

接偶正分壮。

言欢空月虫,

明勋品宜昌。

依伦先日贱,

洁偶正粉妆。


贾言

架构师说, 用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<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 自动扫描服务,那么这个服务默认是单例,其内部成员是多个线程共享的,如果直接用成员变量是有线程不安全的。


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

  • 无视 SimpleDateFormat 非线程安全

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

 

  • 使用 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. 为失败做设计,如果出问题了有降级应对方案。

贾言

验幻空越重,

命循频异长。

依轮线日简,

接偶正分壮。

言欢空月虫,

明勋品宜昌。

依伦先日贱,

洁偶正粉妆。


贾言

架构师说, 用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<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 自动扫描服务,那么这个服务默认是单例,其内部成员是多个线程共享的,如果直接用成员变量是有线程不安全的。


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

  • 无视 SimpleDateFormat 非线程安全

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

 

  • 使用 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. 为失败做设计,如果出问题了有降级应对方案。

原创粉丝点击
热门问题 老师的惩罚 人脸识别 我在镇武司摸鱼那些年 重生之率土为王 我在大康的咸鱼生活 盘龙之生命进化 天生仙种 凡人之先天五行 春回大明朝 姑娘不必设防,我是瞎子 陕西省住房公积金中心 深圳市住房和建设局 南宁市住房保障和房产管理局 住房公积金是什么 十堰住房公积金查询 广安住房公积金查询 枣庄住房公积金管理中心 南宁市住房公积金查询 陕西住房公积金查询网 南充市住房公积金管理中心 南宁住房公积金 晋中住房公积金查询个人账户 保定住房公积金查询 淮南市住房公积金查询 苏州市住房公积金管理中心 宜春市住房公积金管理中心 住房公积金管理中心电话 福州住房公积金查询 玉林住房公积金查询 住房公积金管理中心地址 烟台住房公积金查询个人账户 个人住房公积金余额查询 成都市住房公积金中心 东莞住房公积金个人帐户查询 西安市住房公积金查询 黄石住房公积金查询 住房公积金客服电话 保定住房公积金 郑州住房公积金 住房公积金咨询电话 泉州市住房公积金个人查询 南充住房公积金查询个人账户 住房公积金电话号码 银川住房公积金查询 淮南住房公积金查询个人账户 毕节住房公积金查询 邯郸市住房公积金管理中心 渭南市住房公积金管理中心 连云港住房公积金 临沂住房公积金查询个人账户 南充住房公积金