设计模式之美 – 通过一段ID生成器代码,学习如何发现代码质量问题

声明:此文章完全是参考【设计模式之美】的文章,并非原创,仅做为学习记录用的.

在前面几节课中,我们学习一些跟重构相关的理论知识,比如:

  • 持续重构
  • 单元测试
  • 代码的可测试性、解耦、编码规范

用一句话总结一下,重构就是发现代码质量问题,并且对其进行优化的过程.

今天,我们就借助一个大家都很熟悉的 ID生成器代码,给大家展示一下重构的大致过程.

1.ID 生成器需求背景介绍

  • ID是什么? (“ID”中文翻译为“标识(Identifier)”。这个概念在生活、工作中随处可见,比如身份证、商品条形码、二维码、车牌号、驾照号。聚焦到软件开发中,ID 常用来表示一些业务信息的唯一标识,比如订单的单号或者数据库中的唯一主键,比如地址表中的 ID 字段(实际上是没有业务含义的,对用户来说是透明的,不需要关注.
    假设你正在参与一个后端业务系统的开发,为了方便在请求出错时排查问题,我们在编写代码的时候会在关键路径上打印日志。某个请求出错之后,我们希望能搜索出这个请求对应的所有日志,以此来查找问题的原因。而实际情况是,在日志文件中,不同请求的日志会交织在一起。如果没有东西来标识哪些日志属于同一个请求,我们就无法关联同一个请求的所有日志。
    这听起来有点像微服务中的调用链追踪。不过,微服务中的调用链追踪是服务间的追踪,我们现在要实现的是服务内的追踪
    )
  • 借鉴微服务调用链追踪的实现思路,我们可以给每个请求分配一个唯一 ID,并且保存在请求的上下文(Context)中,比如,处理请求的工作线程的局部变量中。在 Java 语言中,我们可以将 ID 存储在 Servlet 线程的 ThreadLocal 中,或者利用 Slf4j 日志框架的MDC(Mapped Diagnostic Contexts)来实现(实际上底层原理也是基于线程的ThreadLocal)。每次打印日志的时候,我们从请求上下文中取出请求 ID,跟日志一块输出。这样,同一个请求的所有日志都包含同样的请求 ID 信息,我们就可以通过请求 ID 来搜索同一个请求的所有日志了。
  • 需求背景我们已经讲清楚了,至于具体如何实现整个需求,我就不展开来讲解了。如果你感兴趣的话,可以自己试着去设计实现一下。我们接下来只关注其中生成请求 ID 这部分功能的开发。 

2.一份通用的代码实现

public class IdGenerator {
	private static final Logger logger = LoggerFactory.getLogger(IdGenerator.clas
	
	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 示例如下所示。整个 ID 由三部分组成。

  • 第一部分是本机名的最后一个字段
  • 第二部分是当前时间戳,精确到毫秒
  • 第三部分是 8 位的随机字符串,包含大小写字母和数字

尽管这样生成的 ID 并不是绝对唯一的,有重复的可能,但事实上重复的概率非常低。对于我们的日志追踪来说,极小概率的 ID 重复是完全可以接受的.

103-1577456311467-3nR3Do45
103-1577456311468-0wnuV5yw
103-1577456311468-sdrnkFxN
103-1577456311468-8lwk0BP0

大家可以看一下,这份代码是否有问题?

3.如何发现代码质量问题

从大处着眼的话,我们可以参考之前讲过的代码质量评判标准,看这段代码是否可读、可扩展、可维护、灵活、简洁、可复用、可测试等等。落实到具体细节,我们可以从以下几个方面来审视代码。

  • 目录设置是否合理模块划分是否清晰、代码结构是否满足“高内聚、松耦合”?
  • 是否遵循经典的设计原则和设计思想(SOLID、DRY、KISS、YAGNI、LOD 等)?
  • 设计模式是否应用得当?是否有过度设计?
  • 代码是否容易扩展?如果要添加新功能,是否容易实现?
  • 代码是否可以复用?是否可以复用已有的项目代码或类库?是否有重复造轮子?
  • 代码是否容易测试?单元测试是否全面覆盖了各种正常和异常的情况?
  • 代码是否易读?是否符合编码规范(比如命名和注释是否恰当、代码风格是否一致等)?

以上是一些通用的关注点,可以作为常规检查项,套用在任何代码的重构上。除此之外,我们还要关注代码实现是否满足业务本身特有的功能和非功能需求。

  • 代码是否实现了预期的业务需求?
  • 逻辑是否正确?是否处理了各种异常情况?
  • 日志打印是否得当?是否方便 debug 排查问题?
  • 接口是否易用?是否支持幂等、事务等?
  • 代码是否存在并发问题?是否线程安全?
  • 性能是否有优化空间,比如,SQL、算法是否可以优化?
  • 是否有安全漏洞?比如输入输出校验是否全面?

对照以上知识,我们来检查下有那些不合理的地方

  • 可测试性不好并且没有编写单元测试代码
  • 代码不易读
  • 逻辑问题,未处理hostName异常的情况
  • 性能问题,获取主机名会比较耗时,随机范围不合理
  • 三个if是否重复,考虑优化

4.重构

    重构的原则和步骤:稳定为前提,循序渐进重构,我们分成四次重构完成

  1. 提高代码的可读性
  2. 提高代码的可测试性
  3. 编写完善的单元测试
  4. 所有重构完成之后添加注释    

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

 要解决的问题

  • hostName变量使用问题
  • 随机数生成抽离
  • 三个if优化
  • 面向接口编程    

public interface IdGenerator {
	String generate();
}
public interface LogTraceIdGenerator extends IdGenerator {
}
public class RandomIdGenerator implements IdGenerator {
	private static final Logger logger = LoggerFactory.getLogger(RandomIdGenerato
	@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();

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

要解决的问题

  • generate() 函数的代码实现依赖运行环境(本机名)、时间函数、随机函数,所以generate() 函数本身的可测试性也不好

public class RandomIdGenerator implements IdGenerator {
	private static final Logger logger = LoggerFactory.getLogger(RandomIdGenerato
	
	@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);
	}
}

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

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

  • public String generate();
  • private String getLastfieldOfHostName();
  • protected String getLastSubstrSplittedByDot(String hostName);
  • protected String generateRandomAlphameric(int length);

我们先来看后两个函数。这两个函数包含的逻辑比较复杂,是我们测试的重点。而且,在上一步重构中,为了提高代码的可测试性,我们已经这两个部分代码跟不可控的组件(本机名、随机函数、时间函数)进行了隔离。所以,我们只需要设计完备的单元测试用例即可

public class RandomIdGeneratorTest {
	@Test
	public void testGetLastSubstrSplittedByDot() {
		RandomIdGenerator idGenerator = new RandomIdGenerator();
		String actualSubstr = idGenerator.getLastSubstrSplittedByDot("field1.field2
		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或空字符串的情况
	// 这部分优化留在第36、37节课中讲解
	@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' <
		}
	}
	// 此单元测试会失败,因为我们在代码中没有处理length<=0的情况
	// 这部分优化留在第36、37节课中讲解
	@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 函数。虽然逻辑比较简单,最好还是测试一下。但是,它依赖主机名、随机函数、时间函数,我们该如何测试呢?需要 mock 这些函数的实现吗?
实际上,这要分情况来看。我们前面讲过,写单元测试的时候,测试对象是函数定义的功能,而非具体的实现逻辑。这样我们才能做到,函数的实现逻辑改变了之后,单元测试用例仍然可以工作。那 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,测试代码运行是否符合预期

最后,我们来看下 getLastfieldOfHostName() 函数。实际上,这个函数不容易测试,因为它调用了一个静态函数(InetAddress.getLocalHost().getHostName();),并且这个静态函数依赖运行环境。但是,这个函数的实现非常简单,肉眼基本上可以排除明显的bug,所以我们可以不为其编写单元测试代码。毕竟,我们写单元测试的目的是为了减少代码 bug,而不是为了写单元测试而写单元测试

8.第四轮重构:添加注释

对于如何写注释,你可以参看我们在第 31 节课中的讲解。总结一下,主要就是写清楚:做什么、为什么、怎么做、怎么用,对一些边界条件、特殊情况进行说明,以及对函数输入、输出、异常进行说明

/**
* Id Generator that is used to generate random IDs.
*
* 

* The IDs generated by this class are not absolutely unique,
* but the probability of duplication is very low.
*/
public class RandomIdGenerator implements IdGenerator {

	private static final Logger logger = LoggerFactory.getLogger(RandomIdGenerato
	
	/**
	* Generate the random ID. The IDs may be duplicated only in extreme situatio
	*
	* @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 obtain
	*/
	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}
	*/
	@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) {
		//...
	}
}

9.重构总结

  • 重构的目的是解决问题,并不是为了重构而重构,在没有遇到问题或者没有目的的时候不可轻易重构
  • 重构以稳定为大前提,循序渐进,小步快跑的方式来进行
  • 我们要对代码质量有所追求,不能只是凑活能用就好。花点心思写一段高质量的代码,比写 100 段凑活能用的代码,对我们的代码能力提高更有帮助
  • 知其然知其所以然,了解优秀代码设计的演变过程,比学习优秀设计本身更有价值。知道为什么这么做,比单纯地知道怎么做更重要,这样可以避免你过度使用设计模式、思想和原则

最后给大家留下一个思考:能不能提前做好而避免重构呢?



此条目发表在 网站开发 分类目录。将固定链接加入收藏夹。

设计模式之美 – 通过一段ID生成器代码,学习如何发现代码质量问题》有 5 条评论

  1. xtgxiso 说:

    什么是幂等性?
    在分布式系统中,多系统之间接口调用的时候我们经常听到幂等性,那么幂等是啥?幂等最早是一个数学概念,在数学与计算机学中幂等(Idempotence) 是指相同参数重复执行,并能获得相同结果的函数。这里还有个公式:f(f(x)) = f(x)。
    在编程领域里通俗说是指一个操作重复执行N次得到的结果与执行一次是相等的
    举例子更加容易理解
        1. 前端重复提交选中的数据,应该后台只产生对应这个数据的一个反应结果。
        2. 我们发起一笔付款请求,应该只扣用户账户一次钱,当遇到网络重发或系统bug重发,也应该只扣一次钱;
        3. 创建业务订单,一次业务请求只能创建一个,如果遇到了请求超时或服务器内部错误时,客户端可能会尝试重发请求,创建多个就会出大问题。

      所以幂等对于确保数据正确性具有重大的意义

  2. xtgxiso 说:

    获取主机性能
    可以从环境变量中获取

  3. xtgxiso 说:

    为什么说静态方法会影响代码的可测试性?
    单个的静态方法如果没有其他依赖的话,非常容易测试,但如果静态方法A又调用了静态方法B,因为B的不可替换性,我们无法单独测试静态方法A。如果整个系统引入了很多静态方法,那么这个系统的可测试性就会大大降低。简言之,静态方法是面向过程的,是一种紧耦合,从而降低了可测试性
    不好mock

  4. xtgxiso 说:

    1.split效率较低
    2.随机字符的生成可用固定数组随机取数:
    int count = 0;
    String random = “”;
    char[] doc = { ‘A’, ‘B’, ‘C’, ‘D’, ‘a’, ‘b’, ‘c’,'d’,’1′,’2′,’3′ };
    while (count < 8) {
    int index = (int) (Math.random() * doc.length);
    random = random + (doc[index]);
    count++;
    }
    System.out.println(random);

发表评论