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

一份“能用”的代码实现

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

如何发现代码质量问题?

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

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

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

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

现在,对照业务本身无关的、通用的代码质量关注点,我们来看一下,上面的代码有哪些问题:
(1)首先,IdGenerator 的代码比较简单,只有一个类,所以,不涉及目录设置、模块划分、代码结构问题,也不违反基本的 SOLID、DRY、KISS、YAGNI、LOD 等设计原则。它没有应用设计模式,所以也不存在不合理使用和过度设计的问题。

(2)其实,IdGenerator设计成了实现类而非接口,调用者直接依赖实现而非接口,违反基于接口而非实现编程的设计思想。实际上,将 IdGenerator 设计成实现类,而不定义接口,问题也不大。如果哪天 ID 生成算法改变了,我们只需要直接修改实现类的代码就可以。但是,如果项目中需要同时存在两种ID生成算法,也就是要同时存在两个IdGenerator实现类。比如,我们需要将这个框架给更多的系统来使用。系统在使用的时候,可以灵活的选择它需要的生成算法。这个时候,我们就需要将IdGenerator 定义为接口,并且为不同的生成算法定义不同的实现类

(3)再次,把IdGenerator的generate()函数定义为静态函数,会影响使用该函数的代码的可测试性。同时,generate()函数的代码实现依赖运行环境(本机名)、施加函数、随机函数,所以generate()函数本身的可测试性也不好,需要做比较大的重构。除此之外,还需要补充一份单元测试代码

(4)最后,虽然 IdGenerator 只包含一个函数,并且代码行数也不多,但代码的可读性并不好。特别是随机字符串生成的那部分代码,一方面,代码完全没有注释,生成算法比较难读懂,另一方面,代码里有很多魔数,严重影响代码的可读性。在重构的时候,我们需要重点提高这部分代码的可读性。

然后,我们对照业务本身的功能和非功能需求,重新审视一下上面的代码。

(1)虽然生成的 ID 并非绝对的唯一,但是对于追踪打印日志来说,不过,获取 hostName 这部分代码逻辑貌似有点问题,并未处理“hostName 为空”的情况。除此之外,尽管代码中针对获取不到本机名的情况做了异常处理,但是小王对异常的处理是在 IdGenerator 内部将其吐掉,然后打印一条报警日志,并没有继续往上抛出。这样的异常处理是否得当呢?应该需要继续抛出,因为在实际的业务开发中,会有对应的异常处理器,抛出可以让调用者明白哪出错了,而不是只是简单的打印日志。

(2)上面代码的日志打印得当,日志描述能够准确反应问题,方便 debug,并且没有过多的冗余日志。IdGenerator 只暴露一个 generate() 接口供使用者使用,接口的定义简单明了,不存在不易用问题。generate() 函数代码中没有涉及共享变量,所以代码线程安全,多线程环境下调用 generate() 函数不存在并发问题。

(3)每次生成 ID 都需要获取本机名,获取主机名会比较耗时,所以,这部分可以考虑优化一下。还有,randomAscii 的范围是 0~122,但可用数字仅包含三段子区间(09,az,A~Z),极端情况下会随机生成很多三段区间之外的无效数字,需要循环很多次才能生成随机字符串,所以随机字符串的生成算法也可以优化一下。

(4)在 generate() 函数的 while 循环里面,三个 if 语句内部的代码非常相似,而且实现稍微有点过于复杂了,实际上可以进一步简化,将这三个 if 合并在一起

如何将代码从“能用”重构为“好用”

重构代码的时候要循序渐进、小步快跑。每次改动一点点,改好之后,再进行下一轮的优化,保证每次对代码的改动不会过大,能在很短的时间内完成。因此,分四次重构来完成:

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

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

首先,我们要解决最明显、最急需改进的代码可读性问题。具体有下面几点:

  • hostName 变量不应该被重复使用,尤其当这两次使用时的含义还不同的时候
  • 将获取 hostName 的代码抽离出来,定义为 getLastfieldOfHostName() 函数
  • 删除代码中的魔法数,比如,57、90、97、122
  • 将随机数生成的代码抽离出来,定义为 generateRandomAlphameric() 函数
  • generate() 函数中的三个 if 逻辑重复了,且实现过于复杂,我们要对其进行简化
  • 对 IdGenerator 类重命名,并且抽象出对应的接口

我们抽象出两个接口,一个是 IdGenerator,一个是LogTraceIdGenerator,LogTraceIdGenerator 继承 IdGenerator。实现类实现接口IdGenerator,命名为 RandomIdGenerator、SequenceIdGenerator 等。这样,实现类可以复用到多个业务模块中

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

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

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

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

对于第一点我们已经在第一轮重构中解决了。我们将 RandomIdGenerator 类中的generate()静态函数重新定义成了普通函数。调用者可以通过依赖注入的方式,在外部创建号 RandomIdGenerator 对象后注入到自己的代码中,从而解决静态函数调用影响代码可测试性的问题。

对于第二点,我们需要在第一轮重构的基础之上再进行重构。重构之后的代码如下所示,主要包括以下几个代码改动。

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

上面提到,打印日志的 Logger 对象被定义为 static final 的,并且在类内部创建,这是否影响到代码的可测试性?是否应该将Logger对象通过依赖注入的方式注入到类中呢?

  • 依赖注入之所以能提高代码的可测试性,主要是因为,通过这样的方式我们能轻松的用mock对象替换依赖的真实对象。那我们为什么要mock这个对象呢?这是因为,这个对象参数逻辑执行行(比如,我们要依赖它输出的数据做后续的计算)但又不可控。对于Logger对象来说,我们只往里写入数据,并不读取数据,不参与业务逻辑的执行,不会影响代码逻辑的正确性。所以,我们没有必要mock Logger对象
  • 除此之外,一些只是为了存储数据的值对象,比如String、Map、UseVo,我们也没必要通过依赖注入的方式来创建,直接来类中通过new创建就可以了

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

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

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

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

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或空字符串的情况
	@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的情况
	@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种不同的单元测试。

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

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

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

当然,如果你真的想要对它进行测试,我们也是有办法的。一种办法是使用更加高级的测试框架。比如 PowerMock,它可以 mock 静态函数。另一种方式是将获取本机名的逻辑再封装为一个新的函数。不过,后一种方法会造成代码过度零碎,也会稍微影响到代码的可读性,这个需要你自己去权衡利弊来做选择。

第四轮重构:添加注释

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

如何处理异常

平时进行软件设计开发的时候,我们除了要保证正常情况下的逻辑运行正确之外,还需要编写大量额外的代码,来处理有可能出现的异常情况,以保证代码在任何情况下,都在我们的掌控之内,不会出现非预期的运行结果。程序的 bug 往往都出现在一些边界条件和异常情况下,所以说,异常处理得好坏直接影响了代码的健壮性。全面、合理地处理各种异常能有效减少代码 bug,也是保证代码质量的一个重要手段。

重构 generate() 函数

首先,我们来看,对于 generate() 函数,如果本机名获取失败,函数返回什么?这样的返回值是否合理?

public String generate() {
	String substrOfHostName = getLastFiledOfHostName();
	long currentTimeMillis = System.currentTimeMillis();
	String randomString = generateRandomAlphameric(8);
	String id = String.format("%s-%d-%s",
	substrOfHostName, currentTimeMillis, randomString);
	return id;
}

ID 由三部分构成:本机名、时间戳和随机数。时间戳和随机数的生成函数不会出错,唯独主机名有可能获取失败。在目前的代码实现中,如果主机名获取失败,substrOfHostName 为 NULL,那 generate() 函数会返回类似“null-16723733647-83Ab3uK6”这样的数据。如果主机名获取失败,substrOfHostName 为空字符串,那generate() 函数会返回类似“-16723733647-83Ab3uK6”这样的数据。

在异常情况下,返回上面两种特殊的 ID 数据格式,这样的做法是否合理呢?这个其实很难讲,我们要看具体的业务是怎么设计的。不过,更倾向于明确地将异常告知调用者。所以,这里最好是抛出受检异常,而非特殊值。

按照这个设计思路,我们对 generate() 函数进行重构。重构之后的代码如下所示:

public String generate() throws IdGenerationFailureException {
	String substrOfHostName = getLastFiledOfHostName();
	if (substrOfHostName == null || substrOfHostName.isEmpty()) {
		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;
}

重构 getLastFiledOfHostName() 函数

对于 getLastFiledOfHostName() 函数,是否应该将 UnknownHostException 异常在函数内部吞掉(try-catch 并打印日志),还是应该将异常继续往上抛出?如果往上抛出的话,是直接把 UnknownHostException 异常原封不动地抛出,还是封装成新的异常抛出?

private String getLastFiledOfHostName() {
	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;
}

现在的处理方式是当主机名获取失败的时候,getLastFiledOfHostName() 函数返回 NULL值。是返回NULL还是异常对象,要看获取不到数据时正常行为,还是异常行为。获取主机名失败后会影响后继逻辑的处理,并不是我们期望的,所以,它是一种异常行为。这里最好是抛出异常,而非返回NULL值。

至于是直接将 UnknownHostException 抛出,还是重新封装成新的异常抛出,要看函数跟异常是否有业务相关性。getLastFiledOfHostName() 函数用来获取主机名的最后一个字段,UnknownHostException 异常表示主机名获取失败,两者算是业务相关,所以可以直接将 UnknownHostException 抛出,不需要重新包裹成新的异常。

按照上面的设计思路,我们对 getLastFiledOfHostName() 函数进行重构。重构后的代码如下所示:

private String getLastFiledOfHostName() throws UnknownHostException{
	String substrOfHostName = null;
	String hostName = InetAddress.getLocalHost().getHostName();
	substrOfHostName = getLastSubstrSplittedByDot(hostName);
	return substrOfHostName;
}

getLastFiledOfHostName() 函数修改之后,generate() 函数也要做相应的修改。我们需要在 generate() 函数中,捕获 getLastFiledOfHostName() 抛出的UnknownHostException 异常。当我们捕获到这个异常之后,应该怎么处理呢?

按照之前的分析,ID 生成失败的时候,我们需要明确地告知调用者。所以,我们不能在generate() 函数中,将 UnknownHostException 这个异常吞掉。那我们应该原封不动地抛出,还是封装成新的异常抛出呢?

我们选择后者。在 generate() 函数中,我们需要捕获 UnknownHostException 异常,并重新包裹成新的异常 IdGenerationFailureException 往上抛出。之所以这么做,有下面三个原因。

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

按照上面的设计思路,我们对 generate() 的函数再次进行重构。重构后的代码如下所示:

public String generate() throws IdGenerationFailureException {
	String substrOfHostName = null;
	try {
		substrOfHostName = getLastFiledOfHostName();
	} 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;
}

重构 getLastSubstrSplittedByDot() 函数

对于 getLastSubstrSplittedByDot(String hostName) 函数,如果 hostName 为 NULL或者空字符串,这个函数应该返回什么?

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

理论上讲,参数传递的正确性应该有程序员来保证,我们无需做 NULL 值或者空字符串的判断和特殊处理。调用者本不应该把 NULL 值或者空字符串传递给getLastSubstrSplittedByDot() 函数。如果传递了,那就是 code bug,需要修复。但是,话说回来,谁也保证不了程序员就一定不会传递 NULL 值或者空字符串。那我们到底该不该做 NULL 值或空字符串的判断呢?

  • 如果函数是 private 类私有的,只在类内部被调用,完全在你自己的掌控之下,自己保证在调用这个 private 函数的时候,不要传递 NULL 值或空字符串就可以了。所以,我们可以不在 private 函数中做 NULL 值或空字符串的判断。
  • 如果函数是 public 的,你无法掌控会被谁调用以及如何调用,为了尽可能提高代码的健壮性,我们最好是在 public 函数中做 NULL 值或空字符串的判断。

那getLastSubstrSplittedByDot() 是 protected 的,既不是 private 函数,也不是 public 函数,那要不要做 NULL 值或空字符串的判断呢?

  • 之所以将它设置为 protected,是为了方便写单元测试。不过,单元测试可能要测试一些corner case,比如输入是 NULL 值或者空字符串的情况。
  • 所以,这里我们最好也加上NULL 值或空字符串的判断逻辑。虽然加上有些冗余,但多加些检验总归不会错的。

按照这个设计思路,我们对 getLastSubstrSplittedByDot() 函数进行重构。重构之后的代码如下所示:

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

按照上面讲的,我们在使用这个函数的时候,自己也要保证不传递 NULL 值或者空字符串进去。所以,getLastFiledOfHostName() 函数的代码也要作相应的修改。修改之后的代码如下所示:

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

重构 generateRandomAlphameric() 函数

对于 generateRandomAlphameric(int length) 函数,如果 length < 0 或 length = 0,这个函数应该返回什么?

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

我们先来看 length < 0 的情况。生成一个长度为负值的随机字符串是不符合常规逻辑的,是一种异常行为。所以,当传入的参数 length < 0 的时候,我们抛出IllegalArgumentException 异常。

我们再来看 length = 0 的情况。length = 0 是否是异常行为呢?这就看你自己怎么定义了。我们既可以把它定义为一种异常行为,抛出 IllegalArgumentException 异常,也可以把它定义为一种正常行为,让函数在入参 length = 0 的情况下,直接返回空字符串。不管选择哪种处理方式,最关键的一点是,要在函数注释中,明确告知 length = 0 的情况下,会返回什么样的数据。

重构之后的 RandomIdGenerator 代码

public class RandomIdGenerator implements IdGenerator {
	private static final Logger logger = LoggerFactory.getLogger(RandomIdGenerato

	@Override
	public String generate() throws IdGenerationFailureException {
		String substrOfHostName = null;
		try {
			substrOfHostName = getLastFiledOfHostName();
		} 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;
	}


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

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



	@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);
	  }
	}
  • 0
    点赞
  • 0
    收藏
    觉得还不错? 一键收藏
  • 0
    评论

“相关推荐”对你有帮助么?

  • 非常没帮助
  • 没帮助
  • 一般
  • 有帮助
  • 非常有帮助
提交
评论
添加红包

请填写红包祝福语或标题

红包个数最小为10个

红包金额最低5元

当前余额3.43前往充值 >
需支付:10.00
成就一亿技术人!
领取后你会自动成为博主和红包主的粉丝 规则
hope_wisdom
发出的红包
实付
使用余额支付
点击重新获取
扫码支付
钱包余额 0

抵扣说明:

1.余额是钱包充值的虚拟货币,按照1:1的比例进行支付金额的抵扣。
2.余额无法直接购买下载,可以购买VIP、付费专栏及课程。

余额充值