【Code】《代码整洁之道》笔记-Chapter16-重构SerialDate

最新推荐文章于 2025-10-27 10:10:12 发布

原创

最新推荐文章于 2025-10-27 10:10:12 发布 · 1.2k 阅读

19 ·

CC 4.0 BY-SA版权

文章标签：

#代码规范

第16章　重构SerialDate

如果你找到JCommon类库，深入该类库，其中有个名为org.jfree.date的程序包。在该程序包中，有个名为SerialDate的类，我们即将剖析这个类。

SerialDate的作者是David Gilbert。David显然是一位经验丰富、能力很强的程序员。如我们将看到的，他在代码中展示了极高的专业性和原则性。无论怎么说，SerialDate都是“好代码”，而我将把它撕成碎片。

这并非恶意的行为，我也不认为自己比David强许多，有权对他的代码说三道四。其实，如果你看过我的代码，我敢说你也会发现好些该埋怨的东西。

不，这也并非傲慢无礼的行为。我所要做的，只是一种专业眼光的检视，不多也不少，那是我们都该坦然接受的做法。那是我们应该欢迎别人对自己做的事。只有通过这样的批评，我们才能学到东西。医生就是这样做的，飞行员就是这样做的，律师就是这样做的，我们程序员也需要学习如何这样做。

多说一句关于David Gilbert的事：David不仅是一位优秀的程序员，他还有着将代码免费呈献给社区的勇气和好心。他公开代码，让所有人都能看到，邀请大众使用并审查。做得真好！

SerialDate（见代码清单B-1）是一个用Java呈现日期的类。为什么在Java已经有java.util.Date和java.util.Calendar的情况下，还需要一个呈现日期的类呢？作者编写这个类，是为了解除我自己也常感到的痛苦。在开放的Javadoc（第67行）中，他很好地解释了原因。我们可以质疑他的初衷，但我的确有处理这个问题的需要，而且我也欢迎有一个关乎日期甚于关乎时间的类存在。

16.1　首先，让它能工作

在一个名为SerialDateTests的类（见代码清单B-2）中，有一些单元测试。测试都通过了，但不幸的是，快览一遍测试，发现它们并没有测试所有东西[T1]。例如，用“查找使用”搜索方法MonthCodeToQuarter（第356行），会发现没有被用过[F4]。因此，单元测试并没有测试这个方法。

所以，我用Clover来检查单元测试覆盖了哪些代码。Clover报告说，在SerialDate的185个可执行语句中，单元测试只执行了91个（约50%）[T2]。覆盖图看起来像是一床满是补丁的棉被，整个类上布满大块的未执行代码。

我的目标是完整地理解和重构这个类，如果没有好得多的测试覆盖率，就达不到目标。所以，我完全重起炉灶编写了自己的单元测试（见代码清单B-4）。

在阅读这些测试时，你可以看到，其中许多注释掉了。这些测试不能通过。它们代表了我以为SerialDate应该有的行为。在我重构SerialDate时，也将让这些测试通过。

即便有些测试被注释掉，Clover也还是会报告新的单元测试执行了185个可执行语句中的170个（92%）。这样就好多了，而且我想我们可以把这个覆盖率提高些。

前几个注释掉的测试（第23～63行）是我一厢情愿。程序并没有设计为通过这些测试，但对我来说它们代表的行为显而易见[G2]。我不太确定StringToWeekdayCode方法为何要写成那样，不过既然它已经在那儿，显然不该是区分大小写的。编写这些测试是区区小事[T3]，通过测试更加容易。我只修改了第259行和第263行，就能使用equalsIgnoreCase了。

我注释掉了第32行和第45行的测试，因为我不太明确是否应该支持tues和thurs缩写。

第153行和第154行的测试不能通过。显然，它们本该通过[G2]。我们可以轻易地修正，只要对stringToMonthCode作出以下修改就行，对于第163行和第213行的测试也一样。

457         if ((result < 1) || (result > 12)) {
              result = -1;
458           for (int i = 0; i < monthNames.length; i++) {
459             if (s.equalsIgnoreCase(shortMonthNames[i])) {
460               result = i + 1;
461               break;
462             }
463             if (s.equalsIgnoreCase(monthNames[i])) {
464               result = i + 1;
465               break;
466             }
467           }
468         }

第318行注释掉的测试暴露了getFollowingDayOfWeek方法中的一个缺陷（第672行）。2004年12月25日是周六。下一个周六是2005年1月1日。然而，运行测试时，会看到getFollowingDayOfWeek返回12月25日之后的周六还是12月25日。显然这不对[G3] [T1]。我们看到问题在第685行。那是个典型的边界条件错误[T5]。应该是这样：

685         if (baseDOW >= targetWeekday) {

很有意思，这个函数是之前一次修改的结果。修改记录（第43行）显示，getPrevious- DayOfWeek、getFollowingDayOfWeek和getNearestDayOfWeek中的“缺陷”已被修正[T6]。

测试getNearestDayOfWeek（第705行）的单元测试testGetNearestDayOfWeek（第329行）之前的版本不像现在一样没有遗漏。我添加了大量测试用例，因为初始的测试用例并没有全部通过[T6]。查看哪些测试用例被注释掉，你可以看到失败的模式，这很有启发。如果最近的日期是在未来，算法就会失败。显然存在某种边界条件错误[T5]。

Clover汇报的测试覆盖模式也很有趣[T8]。第719行根本没有执行！这意味着第718行的if语句总是得到false的结果。没错，看一眼代码就知道是这样。变量adjust总是为负，所以不会大于或等于4。所以，算法错了。

正确的算法如下所示：

int delta = targetDOW - base.getDayOfWeek();
int positiveDelta = delta + 7;
int adjust = positiveDelta % 7;
if (adjust > 3)
  adjust -= 7;

return SerialDate.addDays(adjust, base);

最后，只要简单地抛出IllegalArgumentException异常而不是从weekInMonthToString和relativeToString返回错误字符串，第417行和第429行的测试就能通过。

做出这些修改后，所有的单元测试都通过了，我确信SerialDate现在可以工作。是时候让它“做对”了。

16.2　让它做对

我们将从头到尾遍历SerialDate，同时加以改进。尽管在本章的讨论中你看不到这个过程，在每次做修改后，我还是要运行全部JCommon单元测试，包括我为SerialDate改进的那些单元测试。所以，后面你看到的所有修改，对于JCommon都是可工作的。

从第1行开始，我看到大量有关许可、版权、作者和修改历史的注释。我明白，的确有些法律事宜要说明，所以版权和许可信息应该保留。另外，修改历史是产生于19世纪60年代的古董，现今源代码控制工具可以帮我们做到这个。应该删掉修改历史[C1]。

从第61行开始的导入列表应该通过使用java.text.*和java.util.*来缩短。[J1]

Javadoc的HTML格式化工作（第67行）令我畏惧。一个源文件里面有多种语言，我有点发怵。这条注释有4种语言：Java、英文、Javadoc和html[G1]。有那么多语言，注释就很难直截了当。例如，生成Javadoc后，第71行和第72行原本很好的位置就丢失了，而且谁想在源代码中看到<ul>和<li>这样的东西呢？更好的策略可能是用<pre>标签把整个注释部分包围起来，这样，对于源代码的格式化只会限于Javadoc之内（更好的解决方案是让Javadoc不对注释做格式化，这样注释在代码和文档中就会是同一种样式）。

第86行是类声明。这个类为何要命名为SerialDate呢？Serial一词有什么妙处吗？是不是因为该类派生自Serializable？看来不是这样的。

别猜了，我知道为什么（或者我认为自己知道）要用Serial一词。线索就在第98行和第101行的常量SERIAL_LOWER_BOUND和SERIAL_UPPER_BOUND。更好的线索在从第830行开始的注释中。该类被命名为SerialDate，是因为它用“序列数”（serial number）来实现，该序列数恰好是从1899年12月30日后的天数。

对此我有两个问题。首先，术语“序列数”并不真对。可能有点诡辩，但其呈现方式却更接近相对偏移。术语“序列数”更多地用于产品版本标识，而非日期标识。我没发现这个名称特别有描述力[N1]。更有描述力的术语大概是“顺序”（ordinal）。

第二个问题更突出。名称SerialDate暗示了一种实现。该类是一个抽象类，没必要暗示任何有关实现的事。实际上，没理由隐藏实现！我发现这个名称放在了不正确的抽象层级上[N2]。以我之见，该类的名称应该就是简单的Date。

不幸的是，Java类库里面有太多名为Date的类了，所以这大概也不是最好的名称。因为这个类关于日期而非时间，所以我想将其命名为Day，但Day这个名字也在多处被滥用。最后，我选了DayDate作为最佳折中方案。

从现在起，我