第二轮重构：提高代码可测试性
关于代码可测试性的问题，主要包含下面两个方面：
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代码的时候其实也能按这个方法流程来。
