代码重构实战:实现一个ID生成器

所谓重构,就是发现代码质量问题,并且对其进行优化的过程。今天借助一个ID生成器的代码来实际展示一下重构的大概过程。

需求背景介绍

ID生成器在每个系统中几乎都可以见到,假设你正在参与一个后端业务系统的开发,为了方便在请求出错时排查问题,我们编写代码时会在关键路径上打上日志,某个请求出错后我们希望能搜索出对应这个请求的所有日志以方便查找问题原因。但是实际情况是,日志文件中不同的请求日志会交织到一起,如果没有标识哪些日志属于同一个请求,我们就无法关联同一个请求的所有日志。

借助微服务调用链追踪的实现思路,我们可以给每个请求分配一个唯一ID,并且保存在请求的上下文(context)中,比如处理请求的工作线程的局部变量中。在Java语言中,我们可以将ID存储在Servlet线程的ThreadLocal中,或者利用Slf4j日志框架的MDC(Mapped Diagnostic Contexts)来实现(实际上底层原理也是基于线程的ThreadLocal)。每次打印日志的时候,我们从请求上下文中请求ID,跟日志一块输出。这样,同一个请求的所有日志都包含同样的请求ID信息,我们就可以通过请求ID来搜索同一个请求的所有日志了。

一份“能用”的代码实现

下面是一个简单的代码生成器:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Random;


public class IdGenerator {
    private static final Logger logger = LoggerFactory.getLogger(IdGenerator.class);

    public static String generate() {
        String id = "";
        try {
            String hostName = InetAddress.getLocalHost().getHostName();
            String[] tokens = hostName.split("\\.");
            if (tokens.length > 0) {
                hostName = tokens[tokens.length - 1];
            }
            char[] randomChars = new char[8];
            int count = 0;
            Random random = new Random();
            while (count < 8) {
                int randomAscii = random.nextInt(122);
                if (randomAscii >= 48 && randomAscii <= 57) {
                    randomChars[count] = (char) ('0' + (randomAscii - 48));
                    count++;
                } else if (randomAscii >= 65 && randomAscii <= 90) {
                    randomChars[count] = (char) ('A' + (randomAscii - 65));
                    count++;
                } else if (randomAscii >= 97 && randomAscii <= 122) {
                    randomChars[count] = (char) ('a' + (randomAscii - 97));
                    count++;
                }
            }
            id = String.format("%s-%d-%s", hostName,
                    System.currentTimeMillis(), new String(randomChars));
        } catch (UnknownHostException e) {
            logger.warn("Failed to get the host name.", e);
        }

        return id;
    }
}

我们可以看到整个 ID 由三部分组成。第一部分是本机名的最后一个字段。第二部分是当前时间戳,精确到毫秒。第三部分是 8 位的随机字符串,包含大小写字母和数字。尽管这样生成的 ID 并不是绝对唯一的,有重复的可能,但事实上重复的概率非常低。对于我们的日志追踪来说,极小概率的 ID 重复是完全可以接受的。

发现代码质量问题

在确定一段代码有没有质量问题以及按照哪些标准去确定的时候,可以遵循以下的标准:

首先,从大处着手的话,可以看代码是否可读、可扩展、可维护、灵活、简洁、可复用、可测试等,具体可以落实到以下细节:

img

除此之外,我们还要关注代码实现是否满足业务本身特有的功能和非功能需求,一些共性的关注点如下所示:

img

我们可以逐一分析一下:

首先,在常规检查中,由于IdGenerator的功能比较简单,不涉及到目录设置、模块划分、代码结构问题,也不违反基本的SOLID、DRY、KISS、YAGNI、LOD等设计原则,它没有使用设计模式,所以也不存在过度设计的问题。

然后说说可能存在的一些问题:

  • IdGenerator设计成了实现类而非接口,违反了基于接口而非实现编程的原则。目前是没什么问题的,但是一旦要求需要两种ID算法,我们就需要基于接口进行设计了。
  • generate()为静态函数,会影响使用该函数的代码的可测试性。同时其实现依赖运行环境、时间函数、随机函数,所以其本身的可测试性也不好
  • 没有编写单元测试
  • 代码的可读性不好,特别是生成随机字符串的部分,一方面代码没有任何注释,另一方面有很多魔法数,影响代码的可读性。

然后我们再来分析以下业务需求方面:

  • 代码逻辑中并未处理hostname为空的情况
  • 尽管捕获了异常,但是并没有做任何处理,只是打印了一条报警日志,这样的异常处理也不够得当
  • 日志打印得当,日志描述能够准确反应问题,方便 debug,并且没有过多的冗余日志。IdGenerator 只暴露一个 generate() 接口供使用者使用,接口的定义简单明了,不存在不易用问题。generate() 函数代码中没有涉及共享变量,所以代码线程安全,多线程环境下调用 generate() 函数不存在并发问题。
  • 性能方面,ID 的生成不依赖外部存储,在内存中生成,并且日志的打印频率也不会很高,足以应对当前的需求。但是每次生成ID都需要获取本机名,比较耗时,所以可以考虑优化一下。
  • randomAscii 的范围是 0~122,但可用数字仅包含三段子区间(0~9,a~z,A~Z),极端情况下会随机生成很多三段区间之外的无效数字,需要循环很多次才能生成随机字符串,所以随机字符串的生成算法也可以优化一下。

代码重构过程

代码重构的过程应该是循序渐进、小步快跑的,在上一章节中提到的问题,我们可以按照下面的步骤分四次来完成:

  • 第一轮重构:提高代码的可读性
  • 第二轮重构:提高代码的可测试性
  • 第三轮重构:编写完善的单元测试
  • 第四轮重构:所有重构完成后添加注释

第一轮重构:提高代码的可读性

这一轮需要着重解决的问题有:

  • hostName变量不应该被重复使用,尤其当这两次使用的含义还不同的时候,将获取hostName的代码抽离出来
  • 删除代码中的魔法数字
  • 将随机数生成的代码抽离出来
  • generate函数中的三个if逻辑是重复的,我们需要对齐进行简化
  • 对IdGenerator重命名,并且抽象出对应的接口

对于重命名的方式,这里给出一种扩展性比较好的命名方式:

首先我们抽象出两个接口,一个是IdGenerator,一个是LogTraceIdGenerator,LogTraceIdGenerator继承IdGenerator。我们具体的实现类来实现接口LogTraceIdGenerator,可以按照不同的ID生成算法来命名为RandomIdGenerator(随机,类似本文中的代码)、SequenceIdGenerator(有序,比如按照时间戳生成的雪花片算法),这样我们既可以根据具体业务的不同实现不同的ID生成器,对于同一种业务也可以实现不同的算法。

重构后的代码如下:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55

public interface IdGenerator {
  String generate();
}

public interface LogTraceIdGenerator extends IdGenerator {
}

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);
    String id = String.format("%s-%d-%s",
            substrOfHostName, currentTimeMillis, randomString);
    return id;
  }

  private String getLastfieldOfHostName() {
    String substrOfHostName = null;
    try {
      String hostName = InetAddress.getLocalHost().getHostName();
      String[] tokens = hostName.split("\\.");
      substrOfHostName = tokens[tokens.length - 1];
      return substrOfHostName;
    } catch (UnknownHostException e) {
      logger.warn("Failed to get the host name.", e);
    }
    return substrOfHostName;
  }

  private String generateRandomAlphameric(int length) {
    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);
  }
}

//代码使用举例
LogTraceIdGenerator logTraceIdGenerator = new RandomIdGenerator();

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

本轮重构主要解决如下两个问题:

  • generate() 函数定义为静态函数,会影响使用该函数的代码的可测试性;
  • generate() 函数的代码实现依赖运行环境(本机名)、时间函数、随机函数,所以 generate() 函数本身的可测试性也不好。

其中第一个问题我们已经在上一轮解决了。

对于第二个问题,我们主要改动点为:

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

【这里的意思就是说,我们可以将一些调用其他类库或者依赖本机环境的代码单独抽离出来,只测试自己的实现代码的部分,为此而带来的改动就是原本应该是私有的函数要修改其权限级别来保证方法能够被单元测试调用】

重构后的代码如下:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51

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);
    String id = String.format("%s-%d-%s",
            substrOfHostName, currentTimeMillis, randomString);
    return id;
  }

  private String getLastfieldOfHostName() {
    String substrOfHostName = null;
    try {
      String hostName = InetAddress.getLocalHost().getHostName();
      substrOfHostName = getLastSubstrSplittedByDot(hostName);
    } catch (UnknownHostException e) {
      logger.warn("Failed to get the host name.", e);
    }
    return substrOfHostName;
  }

  @VisibleForTesting
  protected String getLastSubstrSplittedByDot(String hostName) {
    String[] tokens = hostName.split("\\.");
    String substrOfHostName = tokens[tokens.length - 1];
    return substrOfHostName;
  }

  @VisibleForTesting
  protected String generateRandomAlphameric(int length) {
    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);
  }
}

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

经过前两轮的重构,代码中存在的明显问题基本上已经解决了,目前类中主要包含以下几个函数:

1
2
3
4
5
6
public String generate();
private String getLastfieldOfHostName();
@VisibleForTesting
protected String getLastSubstrSplittedByDot(String hostName);
@VisibleForTesting
protected String generateRandomAlphameric(int length);

首先,最后两个函数是我们算法的关键部分,所以我们需要重点进行测试,单元测试具体的细节不再赘述,可以参考代码:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50

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

  // 此单元测试会失败,因为我们在代码中没有处理hostName为null或空字符串的情况
  // 这部分优化留在下面的课程中中讲解
  @Test
  public void testGetLastSubstrSplittedByDot_nullOrEmpty() {
    RandomIdGenerator idGenerator = new RandomIdGenerator();
    String actualSubstr = idGenerator.getLastSubstrSplittedByDot(null);
    Assert.assertNull(actualSubstr);

    actualSubstr = idGenerator.getLastSubstrSplittedByDot("");
    Assert.assertEquals("", 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'));
    }
  }

  // 此单元测试会失败,因为我们在代码中没有处理length<=0的情况
  // 这部分优化留在下面的课程中中讲解
  @Test
  public void testGenerateRandomAlphameric_lengthEqualsOrLessThanZero() {
    RandomIdGenerator idGenerator = new RandomIdGenerator();
    String actualRandomString = idGenerator.generateRandomAlphameric(0);
    Assert.assertEquals("", actualRandomString);

    actualRandomString = idGenerator.generateRandomAlphameric(-1);
    Assert.assertNull(actualRandomString);
  }
}

然后是generate函数,这个函数作为我们暴露给外界的public函数,肯定是需要进行测试的。但是这个方法依赖主机名、随机函数、时间函数,我们测试时必须根据实际的情况来确定单元测试的具体写法。我们知道,写**单元测试的时候,测试对象是函数定义的功能,而非具体的实现逻辑。**而我们根据generate函数的具体实现功能可以编写不同的单元测试:

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

最后是getLastfieldOfHostName函数,前面我们说过,这个函数调用了一个依赖运行环境的静态函数,但是其实现由于我们的特意分离而变得特别简单,所以是不需要进行单元测试的。

第四轮重构:添加注释

注释的要点就是写清楚做什么、为什么、怎么做、怎么用,对一些边界条件、特殊情况进行说明,以及对函数输入、输出、异常进行说明。

在这个例子中,可以按照如下的方法进行注释:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54

/**
 * 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 getLastSubstrSplittedByDot(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) {
    //...
  }
}

总结

  1. 即便是非常简单的需求,不同水平的人写出来的代码,差别可能会很大。我们要对代码质量有所追求,不能只是凑活能用就好。花点心思写一段高质量的代码,比写 100 段凑活能用的代码,对你的代码能力提高更有帮助。
  2. 知其然知其所以然,了解优秀代码设计的演变过程,比学习优秀设计本身更有价值。知道为什么这么做,比单纯地知道怎么做更重要,这样可以避免你过度使用设计模式、思想和原则。
  3. 设计思想、原则、模式本身并没有太多“高大上”的东西,都是一些简单的道理,而且知识点也并不多,关键还是锻炼具体代码具体分析的能力,把知识点恰当地用在项目中。
  4. 我经常讲,高手之间的竞争都是在细节。大的架构设计、分层、分模块思路实际上都差不多。没有项目是靠一些不为人知的设计来取胜的,即便有,很快也能被学习过去。所以,关键还是看代码细节处理得够不够好。这些细节的差别累积起来,会让代码质量有质的差别。所以,要想提高代码质量,还是要在细节处下功夫。
updatedupdated2023-06-032023-06-03
加载评论