第二轮重构：提高代码可测试性

关于代码可测试性的问题，主要包含下面两个方面：

generate 函数定义为静态函数，会影响使用该函数的代码的可测试性，第一个问题已经解决，调用时可以在外部创建好IdGenerator然后实现。
generate函数的代码实现依赖运行环境（本机名）、时间函数、随机函数，所以 generate 函数本身的可测试性也不好。

对于第二点，我们需要对RandomIdGenerator 进一步处理：

从 getLastFieldOfHostName函数中，将逻辑比较复杂的那部分代码剥离出来，定义为 getLastSubstrSplitByDot 函数。因为 getLastFieldOfHostName函数依赖本地主机名，所以，剥离出主要代码之后这个函数变得非常简单，可以不用测试。重点测试 getLastSubstrSplitByDot函数即可。
将 generateRandomAlphameric和getLastSubstrSplitByDot这两个函数的访问权限设置为 protected。这样做的目的是，可以直接在单元测试中通过对象来调用两个函数进行测试。
给 generateRandomAlphameric和getLastSubstrSplitByDot两个函数添加 Google Guava 的 annotation @VisibleForTesting。这个 annotation 没有任何实际的作用，只起到标识的作用，告诉其他人说，这两个函数本该是 private 访问权限的，之所以提升访问权限到 protected，只是为了测试，只能用于单元测试中。

重构后代码如下：

package com.example.jackson.service;
import com.sun.org.slf4j.internal.Logger;
import com.sun.org.slf4j.internal.LoggerFactory;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Random;
/**
 * @author tianmaolin004
 * @date 2022/9/24
 */
public class RandomIdGenerator implements LogTraceIdGenerator {
    private static final Logger logger = LoggerFactory.getLogger(RandomIdGenerator.class);
    @Override
    public String generate() {
        String substrOfHostName = getLastFieldOfHostName();
        long currentTimeMillis = System.currentTimeMillis();
        String randomString = generateRandomAlphameric(8);
        return String.format("%s-%d-%s", substrOfHostName, currentTimeMillis, randomString);
    }
    private String getLastFieldOfHostName() {
        try {
            String hostName = InetAddress.getLocalHost().getHostName();
            return getLastSubstrSplitByDot(hostName);
        } catch (UnknownHostException e) {
            logger.warn("Failed to get the host name.", e);
        }
        return null;
    }
    @VisibleForTesting
    protected String getLastSubstrSplitByDot(String hostName) {
        String[] tokens = hostName.split("\\.");
        return tokens[tokens.length - 1];
    }
    @VisibleForTesting
    protected String generateRandomAlphameric(int length) {
        char[] randomChars = new char[length];
        int count = 0;
        Random random = new Random();
        while (count < length) {
            int randomAscii = random.nextInt('z');
            boolean isDigit = randomAscii >= '0' && randomAscii <= '9';
            boolean isUppercase = randomAscii >= 'A' && randomAscii <= 'Z';
            boolean isLowercase = randomAscii >= 'a';
            if (isDigit || isUppercase || isLowercase) {
                randomChars[count] = (char) (randomAscii);
                ++count;
            }
        }
        return new String(randomChars);
    }
}

打印日志的 Logger 对象被定义为 static final 的，并且在类内部创建，这是否影响到代码的可测试性？是否应该将 Logger 对象通过依赖注入的方式注入到类中呢？依赖注入之所以能提高代码可测试性，主要是因为，通过这样的方式能轻松地用 mock 对象替换依赖的真实对象。而mock对象的原因是这个对象参与逻辑执行（比如要依赖它输出的数据做后续的计算）但又不可控。对于 Logger 对象来说，只往里写入数据，并不读取数据，不参与业务逻辑的执行，不会影响代码逻辑的正确性，所以没有必要 mock Logger 对象。除此之外，一些只是为了存储数据的值对象，比如 String、Map、UseVo，也没必要通过依赖注入的方式来创建，直接在类中通过 new 创建就可以

第三轮重构：编写完善的单元测试

经过上面的重构之后，代码存在的比较明显的问题，基本上都已经解决了。现在为代码补全单元测试。RandomIdGenerator 类中有 4 个函数

public String generate();
private String getLastFieldOfHostName(); // 不需要测试
@VisibleForTesting
protected String getLastSubstrSplitByDot(String hostName);
@VisibleForTesting
protected String generateRandomAlphameric(int length);

设计的单元测试如下：

public class RandomIdGeneratorTest {
  @Test
  public void testGetLastSubstrSplittedByDot() {
    RandomIdGenerator idGenerator = new RandomIdGenerator();
    String actualSubstr = idGenerator.getLastSubstrSplittedByDot("field1.field2.field3");
    Assert.assertEquals("field3", actualSubstr);
    actualSubstr = idGenerator.getLastSubstrSplittedByDot("field1");
    Assert.assertEquals("field1", actualSubstr);
    actualSubstr = idGenerator.getLastSubstrSplittedByDot("field1#field2#field3");
    Assert.assertEquals("field1#field2#field3", actualSubstr);
  }
  @Test
  public void testGenerateRandomAlphameric() {
    RandomIdGenerator idGenerator = new RandomIdGenerator();
    String actualRandomString = idGenerator.generateRandomAlphameric(6);
    Assert.assertNotNull(actualRandomString);
    Assert.assertEquals(6, actualRandomString.length());
    for (char c : actualRandomString.toCharArray()) {
         Assert.assertTrue(('0' <= c && c <= '9') || ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'));
    }
  }
}

主要看我们该怎么为generate设计单元测试：写单元测试的时候，测试对象是函数定义的功能，而非具体的实现逻辑。这样我们才能做到，函数的实现逻辑改变了之后，单元测试用例仍然可以工作

比如，针对同一份 generate函数的代码实现，可以有 3 种不同的功能定义，对应 3 种不同的单元测试。

如果把 generate() 函数的功能定义为：“生成一个随机唯一 ID”，那只要测试多次调用 generate() 函数生成的 ID 是否唯一即可
如果把 generate() 函数的功能定义为：“生成一个只包含数字、大小写字母和中划线的唯一 ID”，那不仅要测试 ID 的唯一性，还要测试生成的 ID 是否只包含数字、大小写字母和中划线。
如果把 generate() 函数的功能定义为：“生成唯一 ID，格式为：{主机名 substr}-{时间戳}-{8 位随机数}。在主机名获取失败时，返回：null-{时间戳}-{8 位随机数}”，那不仅要测试 ID 的唯一性，还要测试生成的 ID 是否完全符合格式要求。

总结一下，单元测试用例如何写，关键看如何定义函数。针对 generate函数的前两种定义，不需要 mock 获取主机名函数、随机函数、时间函数等，但对于第 3 种定义，需要 mock 获取主机名函数，让其返回 null，测试代码运行是否符合预期

第四轮重构：增加注释

类或函数包含的逻辑往往比较复杂，单纯靠命名很难清晰地表明实现了什么功能，这个时候就需要通过注释来补充。比如，前面提到的对于 generate函数的 3 种功能定义，就无法用命名来体现，需要补充到注释里面

/**
 * Id Generator that is used to generate random IDs.
 *
 * <p>
 * The IDs generated by this class are not absolutely unique,
 * but the probability of duplication is very low.
 */
public class RandomIdGenerator implements LogTraceIdGenerator {
  private static final Logger logger = LoggerFactory.getLogger(RandomIdGenerator.class);
  /**
   * Generate the random ID. The IDs may be duplicated only in extreme situation.
   *
   * @return an random ID
   */
  @Override
  public String generate() {
    //...
  }
  /**
   * Get the local hostname and
   * extract the last field of the name string splitted by delimiter '.'.
   *
   * @return the last field of hostname. Returns null if hostname is not obtained.
   */
  private String getLastFieldOfHostName() {
    //...
  }
  /**
   * Get the last field of {@hostName} splitted by delemiter '.'.
   *
   * @param hostName should not be null
   * @return the last field of {@hostName}. Returns empty string if {@hostName} is empty string.
   */
  @VisibleForTesting
  protected String getLastSubstrSplitByDot(String hostName) {
    //...
  }
  /**
   * Generate random string which
   * only contains digits, uppercase letters and lowercase letters.
   *
   * @param length should not be less than 0
   * @return the random string. Returns empty string if {@length} is 0
   */
  @VisibleForTesting
  protected String generateRandomAlphameric(int length) {
    //...
  }
}

总的来说主要就是写清楚：做什么、为什么、怎么做、怎么用，对一些边界条件、特殊情况进行说明，以及对函数输入、输出、异常进行说明

第五轮重构：异常处理

函数的运行结果分为两类。一类是预期的结果，也就是函数在正常情况下输出的结果。一类是非预期的结果，也就是函数在异常（或叫出错）情况下输出的结果，非预期结果可以包括这么几种：类似 UnknownHostException 的异常对象，错误码、NULL 值、特殊值（比如 -1）、空对象（比如空字符串、空集合）等

返回NULL值：在多数编程语言中，用 NULL 来表示“不存在”这种语义。对于查找函数来说，数据不存在并非一种异常情况，是一种正常行为，所以返回表示不存在语义的 NULL 值比返回异常更加合理。没找到就是应该返回NULL
返回空对象, 当函数返回的数据是字符串类型或者集合类型的时候，可以用空字符串或空集合替代 NULL 值，来表示不存在的情况。这样，我们在使用函数的时候，就可以不用做 NULL 值判断
抛出异常对象，对于函数抛出的异常，我们有三种处理方法：直接吞掉、直接往上抛出、包裹成新的异常抛出

关于三种抛出异常处理方式

直接吞掉异常并加日志

public void func1() throws Exception1 {
  // ...
}
public void func2() {
  //...
  try {
    func1();
  } catch(Exception1 e) {
    log.warn("...", e); //吞掉：try-catch打印日志
  }
  //...
}

如果 func1() 抛出的异常是可以恢复，且 func2() 的调用方并不关心此异常，我们完全可以在 func2() 内将 func1() 抛出的异常吞掉

原封不动地 re-throw

public void func1() throws Exception1 {
  // ...
}
public void func2() throws Exception1 {//原封不动的re-throw Exception1
  //...
  func1();
  //...
}

如果 func1() 抛出的异常对 func2() 的调用方来说，也是可以理解的、关心的，并且在业务概念上有一定的相关性，可以选择直接将 func1 抛出的异常 re-throw

包装成新的异常 re-throw

public void func1() throws Exception1 {
  // ...
}
public void func2() throws Exception2 {
  //...
  try {
    func1();
  } catch(Exception1 e) {
   throw new Exception2("...", e); // wrap成新的Exception2然后re-throw
  }
  //...
}

如果 func1() 抛出的异常太底层，对 func2() 的调用方来说，缺乏背景去理解、且业务概念上无关，可以将它重新包装成调用方可以理解的新异常，然后 re-throw。接下来解决异常处理的几个问题：

对于 getLastSubstrSplittedByDot(String hostName) 函数，如果 hostName 为 NULL 或者是空字符串，这个函数应该返回什么？
对于 getLastFiledOfHostName() 函数，是否应该将 UnknownHostException 异常在函数内部吞掉（try-catch 并打印日志）？还是应该将异常继续往上抛出？如果往上抛出的话，是直接把 UnknownHostException 异常原封不动地抛出，还是封装成新的异常抛出？
对于 generate() 函数，如果本机名获取失败，函数返回什么？这样的返回值是否合理？
对于 generateRandomAlphameric(int length) 函数，如果 length 小于 0 或者等于 0，这个函数应该返回什么？

入参校验：异常、空值、空对象

理论上讲，参数传递的正确性应该由调用者，我们无需做 NULL 值或者空字符串的判断和特殊处理，但没法保证，所以可以按如下方式规定：

如果函数是 private 类私有的，只在类内部被调用，完全在自己的掌控之下，自己保证在调用这个 private 函数的时候，不要传递 NULL 值或空字符串就可以了。所以，可以不在 private 函数中做 NULL 值或空字符串的判断。
如果函数是 public 的，无法掌控会被谁调用以及如何调用（有可能某个同事一时疏忽，传递进了 NULL 值，这种情况也是存在的），为了尽可能提高代码的健壮性，最好在 public 函数中做 NULL 值或空字符串的判断

事实上除了private作用域，我们最好对参数都做一下校验：

重构 getLastSubstrSplittedByDot(String hostName)

@VisibleForTesting
  protected String getLastSubstrSplittedByDot(String hostName) {
    if (hostName == null || hostName.isEmpty()) {
      throw IllegalArgumentException("..."); //运行时异常
    }
    String[] tokens = hostName.split("\\.");
    String substrOfHostName = tokens[tokens.length - 1];
    return substrOfHostName;
  }

重构 generateRandomAlphameric

@VisibleForTesting
  protected String generateRandomAlphameric(int length) {
    if (length <= 0) {
      throw new IllegalArgumentException("...");
    }
    char[] randomChars = new char[length];
    int count = 0;
    Random random = new Random();
    while (count < length) {
      int maxAscii = 'z';
      int randomAscii = random.nextInt(maxAscii);
      boolean isDigit= randomAscii >= '0' && randomAscii <= '9';
      boolean isUppercase= randomAscii >= 'A' && randomAscii <= 'Z';
      boolean isLowercase= randomAscii >= 'a' && randomAscii <= 'z';
      if (isDigit|| isUppercase || isLowercase) {
        randomChars[count] = (char) (randomAscii);
        ++count;
      }
    }
    return new String(randomChars);
  }
}

这里如果参数校验异常，既可以把它定义为一种异常行为，抛出 IllegalArgumentException 异常，也可以把它定义为一种正常行为，让函数在入参不正确的情况下直接返回空字符串。不管选择哪种处理方式，最关键的一点是，要在函数注释中，明确告知什么异常的情况下，会返回什么样的数据

原封不动地 re-throw 异常

getLastFieldOfHostName() 函数用来获取主机名的最后一个字段，UnknownHostException 异常表示主机名获取失败，两者算是业务相关，所以可以直接将 UnknownHostException 抛出，不需要重新包裹成新的异常，此外，由于调用了getLastSubstrSplittedByDot，所以我们自己也要保证入参的准确性，如下：

private String getLastFieldOfHostName() throws UnknownHostException{
    String substrOfHostName = null;
    String hostName = InetAddress.getLocalHost().getHostName();
    if (hostName == null || hostName.isEmpty()) { // 此处做判断
      throw new UnknownHostException("...");
    }
    substrOfHostName = getLastSubstrSplittedByDot(hostName);
    return substrOfHostName;
 }

包装成新的异常 re-throw 异常

对于generate函数，如果获取失败，最好需要让业务感知到，函数应该抛出异常，而且由于getLastFieldOfHostName抛出了UnknownHostException异常，所以我们需要捕获，但捕获到后最好再封装一层，而不是直接抛出

public String generate() throws IdGenerationFailureException {
    String substrOfHostName = null;
    try {
      substrOfHostName = getLastFieldOfHostName();
    } catch (UnknownHostException e) {
      throw new IdGenerationFailureException("host name is empty.");
    }
    long currentTimeMillis = System.currentTimeMillis();
    String randomString = generateRandomAlphameric(8);
    String id = String.format("%s-%d-%s",substrOfHostName, currentTimeMillis, randomString);
    return id;
  }

调用者在使用 generate() 函数的时候，只需要知道它生成的是随机唯一 ID，并不关心 ID 是如何生成的。也就说是，这是依赖抽象而非实现编程。如果 generate() 函数直接抛出 UnknownHostException 异常，实际上是暴露了实现细节。
从代码封装的角度来讲，不希望将 UnknownHostException 这个比较底层的异常，暴露给更上层的代码，也就是调用 generate() 函数的代码。而且，调用者拿到这个异常的时候，并不能理解这个异常到底代表了什么，也不知道该如何处理。
UnknownHostException 异常跟 generate() 函数，在业务概念上没有相关性。

基于以上考虑，我们需要二次封装异常。

总结一下

用一句话总结一下，重构就是发现代码质量问题，并且对其进行优化的过程。面对一段看起来烂但又不能准确全面的分析的代码时，先依次通过代码诊断的常规检查CheckList和业务检查CheckList将代码问题罗列出来并分类，然后制定多轮重构计划，小步快跑的一轮一轮的重构解决代码问题。这个流程其实也适用于代码CR，在给别人CR代码的时候其实也能按这个方法流程来。

【Java设计模式规范与重构】五重构实战：基于ID生成器case（下）

第二轮重构：提高代码可测试性

第三轮重构：编写完善的单元测试

第四轮重构：增加注释

第五轮重构：异常处理

入参校验：异常、空值、空对象

原封不动地 re-throw 异常

包装成新的异常 re-throw 异常

总结一下

热门文章

最新文章

相关课程

相关电子书

相关实验场景

热门

活动广场

任务中心

开发者评测

高校计划

乘风者计划

训练营

阿里云MVP

话题

直播

下载

镜像站

技术资料

插件

【Java设计模式 规范与重构】 五 重构实战：基于ID生成器case（下）

第二轮重构：提高代码可测试性

第三轮重构：编写完善的单元测试

第四轮重构：增加注释

第五轮重构：异常处理

入参校验：异常、空值、空对象

原封不动地 re-throw 异常

包装成新的异常 re-throw 异常

总结一下

热门文章

最新文章

相关课程

相关电子书

相关实验场景

【Java设计模式规范与重构】五重构实战：基于ID生成器case（下）