代码审查中的一些技术细节问题

作者:盈盈细雨

优秀的代码是优秀应用的根基,通过代码审查可以不断的提高自己的代码能力。但是提高这个事儿,还是得思考的。

一、isDebugEnabled()到底要不要

事情是这样的,在使用sonar进行代码检查的时候,会提示你在调用日志组件时(日志等级为error以下),要预先判断是否对应等级的日志打印功能是否开启,如下图所示:

if (logger.isDebugEnabled()) {
    logger.debug(message);
}

很多文章都会提到这个问题,我也说下这个例子:

//早期的日志
logger.debug("error occurred:" + somMethod());

在这个语句中,无论debug是否开启,待打印的内容,包含字符串拼接和方法都会被执行,在这种情况下,当debug等级日志为关闭的情况下,就存在性能消耗。试想:日志内容没有打印出来,却花了n秒来构造参数,是多么得不偿失的。
因此,sonar提示需要预先判断,用微小的损耗,换取系统可靠的保证,是很明智的。

很显然,日志组件的开发们也发现了这样的缺陷,他们也做了相应的改进:添加了字符串拼接功能

图0:代码审查的那些事儿

slf4j源码片段

//改进后日志
logger.debug("error occurred:{}", description);

在执行对应方法之前先执行判断,这样就不会做无谓的字符串拼接工作了。但是,当参数是一个返回字符串的方法时,仍然无法避免对应方法的执行。

显而易见,需不需要isDebugEnabled()方法成了一个辩证的命题。

二、异常日志堆栈去哪儿了

这个事情是这样的,有一次线上环境出了问题,排查了一圈我除了异常类型和信息,竟找不到任何异常的堆栈信息,简直是奇耻大辱啊。案发现场是这样的:

//底下这个e是捕获的异常
logger.error("error occurred...{}, {}", id, e)

为了防止这类惨案再次发生,我去翻了源码(使用的版本为slf4j-api 1.7.1,源码部分略),还做了一个实验:

//先定义一个异常
Exception e = new RuntimeException("this is a runtime exception");

//情况1:给异常对象分配一个占位符
logger.debug("error occurred... {}, {}", "first string", e);

//调用异常对象的toString方法
logger.debug("error occurred... {}, {}", "third string", e.toString());

//未给异常对象分配占位符
logger.debug("error occurred... {}", "second string", e);

结果果然是还原了案发过程,如下图所示:情况1和2并没有打印出异常的堆栈信息;在对于有占位符的参数对象,消息格式转换会调用对象的toString()方法,所以说情况1和2是相同的。
在希望打印出异常堆栈的情况下(一般情况下,异常堆栈是需要的),不要给Throwable参数添加字符串占位符{},否则不能如愿。

图1:代码审查的那些事儿

前两个日志结果打印

当然,我依然相信日志组件的开发们,肯定也会发现这样的一个问题,于是尝试了最新版本的日志,发现无论是否有占位符,异常的堆栈信息都是能正常打印出来的。不过为了避免踩这个坑,还是建议大家注意占位符的问题。

三、工具类的封装

工具类是对通用方法的封装,通过代码复用来提高编程效率。一般工具类的使用,都是只调用其方法,不涉及到类的任何属性和变量;因此我们看到的工具类中大都如下图所示:

图2:代码审查的那些事儿

java.lang.Math 源码

  • 私有的构造方法
    工具类都是静态方法,无需实例化,有私有的构造方法后,程序就无法通过new Math()来实例化对象,要知道,实例化是有开销的。
  • final修饰
    无法被继承,其方法无法被重写。
    高效的内联调用(也有人说是内嵌、inline)
  • 静态的方法
    无法实例化,肯定只能通过静态方法来访问了。
  • 职责单一 (面向对象的基本原则)
    在项目的代码中见过这样一个工具类:MyUtils.java。光看这个名字,肯定是不知道这个类是做什么用的,看下实现的方法:

图3:代码审查的那些事儿

MyUtils.java的方法

简单的对这些方法进行归类:对象和Json对象的转换、时间格式化、字符串操作,这个类可以拆为三个工具类。

建议:多看看源码,积累工具类,形成自己的工具类库。

四、线程安全对象的使用

线程安全和非线程安全的概念就不多赘述,对于线程安全对象,肯定都非常熟悉,典型的是线程安全集合类:ConcurrentHashMapHashTable
举个例子吧,由于String.java是final修饰的类,造成了很多问题,其中就有一个字符串拼接的问题。在涉及大量字符串拼接方法中,直接使用"str1" + "str2"效率是极其低下的(是因为需要不断的生成新的对象),这时候就需要借助StringBuilderStringBuffer。这两个到底哪个是线程安全的呢?忘记了没关系,点开源码看下说明就好了:

图4:代码审查的那些事儿

StringBuilder.java 源码注释

注释说明了StringBuilder是非线程安全,而StringBuffer是线程安全的(多阅读源码,养成依赖源码而不是百度的习惯。对于会思考的程序员来说是大有裨益的)
只要判断当前的场景,是否需要线程安全就能够合理的使用正确的对象了。

案例一:

public String getXxxMessage(Xxx xxx){
    StringBuffer sb = new StringBuffer();    //在方法内进行拼装,应使用StringBuilder
    sb.append(xxx.getName).append("...")
    //省略部分代码
    return sb.toString()
}

案例二:

//用内存做缓存
private Map map = new ConcurrentHashMap();

public Object getCache(String key){
    ....
}

public void saveCache(Object o){
    ....
}

案例三:

//高并发场景
private Random random = new Random();

public Object saveXX(Xx x){
    int i = random.nextInt(10);   //高并发情况下存在锁竞争引起线程阻塞,影响性能
    doSomething(i);
}

加载余下内容▼

相关文章:

;