第 16 章 Refactoring SerialDate 重构 SerialDate
# 第 16 章 Refactoring SerialDate 重构 SerialDate
If you go to http://www.jfree.org/jcommon/index.php, you will find the JCommon library. Deep within that library there is a package named org.jfree.date. Within that package there is a class named SerialDate. We are going to explore that class.
如果你访问 http://www.jfree.org/jcommon/index.php (opens new window),就能找到 JCommon 类库。深入该类库,其中有个名为 org.jfree.date 的程序包。在该程序包中,有个名为 SerialDate 的类。我们即将剖析这个类。
The author of SerialDate is David Gilbert. David is clearly an experienced and competent programmer. As we shall see, he shows a significant degree of professionalism and discipline within his code. For all intents and purposes, this is “good code.” And I am going to rip it to pieces.
SerialDate 的作者是 David Gilbert。David 显然是位经验丰富、能力足够的程序员。如我们将看到的,他在代码中展示了极高的专业性和原则性。无论怎么说,这都是“好代码”。而我将把它撕成碎片。
This is not an activity of malice. Nor do I think that I am so much better than David that I somehow have a right to pass judgment on his code. Indeed, if you were to find some of my code, I’m sure you could find plenty of things to complain about.
这并非恶意的行为。我也不认为自己比戴维强许多,有权对他的代码说三道四。其实,如果你看过我的代码,我敢说你也会发现好些该埋怨的东西。
No, this is not an activity of nastiness or arrogance. What I am about to do is nothing more and nothing less than a professional review. It is something that we should all be comfortable doing. And it is something we should welcome when it is done for us. It is only through critiques like these that we will learn. Doctors do it. Pilots do it. Lawyers do it. And we programmers need to learn how to do it too.
不,这也并非傲慢无礼的行为。我所要做的,只是一种专业眼光的检视,不多也不少。那是我们都该坦然接受的做法。那是我们应该欢迎别人对自己做的事。只有通过这样的批评,我们才能学到东西。医生就是这样做的。飞行员就是这样做的。律师就是这样做的。我们程序员也需要学习如何这样做。
One more thing about David Gilbert: David is more than just a good programmer. David had the courage and good will to offer his code to the community at large for free. He placed it out in the open for all to see and invited public usage and public scrutiny. This was well done!
多说一句关于 David Gilbert 的事:David 不止是位优秀的程序员。戴维有着将代码免费呈献给社区的勇气和好心。他公开代码,让所有人都能看到,邀请大众使用并审查。做得真好!
SerialDate (Listing B-1, page 349) is a class that represents a date in Java. Why have a class that represents a date, when Java already has java.util.Date and java.util.Calendar, and others? The author wrote this class in response to a pain that I have often felt myself. The comment in his opening Javadoc (line 67) explains it well. We could quibble about his intention, but I have certainly had to deal with this issue, and I welcome a class that is about dates instead of times.
SerialDate(见代码清单 B-1)是一个用 Java 呈现一个日期的类。为什么在 Java 已经有 java.util.Date 和 java.util.Calendar 的时候,还需要一个呈现日期的类呢?作者编写这个类,是为了响应我自己也常感到的痛苦。在开放的 Javadoc(第 67 行)中,他很好地解释了原因。我们可以质疑他的初衷,但我的确有处理这个问题的需要,而且我也欢迎有个关心日期甚于时间的类存在。
# 16.1 FIRST, MAKE IT WORK 首先,让它能工作
There are some unit tests in a class named SerialDateTests (Listing B-2, page 366). The tests all pass. Unfortunately a quick inspection of the tests shows that they don’t test everything [T1]. For example, doing a “Find Usages” search on the method MonthCodeToQuarter (line 334) indicates that it is not used [F4]. Therefore, the unit tests don’t test it.
在一个名为 SerialDateTests 的类(见代码清单 B-2)中,有一些单元测试。测试都通过了。不幸的是,快览一遍测试,发现它们并没有测试所有东西[T1]。例如,用“查找使用”搜索方法 MonthCodeToQuarter(第 334 行),会发现没有被用过[F4]。因此,单元测试并没有测试这个方法。
So I fired up Clover to see what the unit tests covered and what they didn’t. Clover reported that the unit tests executed only 91 of the 185 executable statements in SerialDate (~50 percent) [T2]. The coverage map looks like a patchwork quilt, with big gobs of unexecuted code littered all through the class.
所以,我用 Clover 来检查单元测试覆盖了哪些代码。Clover 报告说,在 SerialDate 的 185 个可执行语句中,单元测试只执行了 91 个(约 50%)[T2]。覆盖图看起来像是一床满是补丁的棉被,整个类上布满大块的未执行代码。
It was my goal to completely understand and also refactor this class. I couldn’t do that without much greater test coverage. So I wrote my own suite of completely independent unit tests (Listing B-4, page 374).
我的目标是完整地理解和重构这个类。没有好得多的测试覆盖率,做不到这个。所以,我完全重起炉灶编写了自己的单元测试(见代码清单 B-4)。
As you look through these tests, you will note that many of them are commented out. These tests didn’t pass. They represent behavior that I think SerialDate should have. So as I refactor SerialDate, I’ll be working to make these tests pass too.
在阅读这些测试时,你可以看到,其中许多注释掉了。这些测试不能通过。它们代表了我以为 SerialDate 应该有的行为。在我重构 SerialDate 时,也将让这些测试通过。
Even with some of the tests commented out, Clover reports that the new unit tests are executing 170 (92 percent) out of the 185 executable statements. This is pretty good, and I think we’ll be able to get this number higher.
即便有些测试被注释掉,Clover 还是报告新的单元测试执行了 185 个可执行语句中的 170 个(92%)。这样就好多了,而且我想我们可以把这个数字提高些。
The first few commented-out tests (lines 23-63) were a bit of conceit on my part. The program was not designed to pass these tests, but the behavior seemed obvious [G2] to me. I’m not sure why the testWeekdayCodeToString method was written in the first place, but because it is there, it seems obvious that it should not be case sensitive. Writing these tests was trivial [T3]. Making them pass was even easier; I just changed lines 259 and 263 to use equalsIgnoreCase.
前几个注释掉的测试(第 23 ~ 63 行)是我一厢情愿。程序并没有设计为通过这些测试,但对我来说它们代表的行为显而易见[G2]。我不太确定 testWeekdayCodeToString 方法为何要写成那样,不过既然它已经在那儿,显然不该是区分大小写的。编写这些测试是区区小事[T3],通过测试更加容易。我只修改了第 259 行和和 263 行,就能使用 equalsIgnoreCase 了。
I left the tests at line 32 and line 45 commented out because it’s not clear to me that the “tues” and “thurs” abbreviations ought to be supported.
我注释掉了第 32 行和第 45 行的测试,因为我不太明确是否应该支持 tues 和 thurs 缩写。
The tests on line 153 and line 154 don’t pass. Clearly, they should [G2]. We can easily fix this, and the tests on line 163 through line 213, by making the following changes to the stringToMonthCode function.
第 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 }
2
3
4
5
6
7
8
9
10
11
12
13
The commented test on line 318 exposes a bug in the getFollowingDayOfWeek method (line 672). December 25th, 2004, was a Saturday. The following Saturday was January 1st, 2005. However, when we run the test, we see that getFollowingDayOfWeek returns December 25th as the Saturday that follows December 25th. Clearly, this is wrong [G3],[T1]. We see the problem in line 685. It is a typical boundary condition error [T5]. It should read as follows:
第 318 行注释掉的测试暴露了 getFollowingDayOfWeek 方法中的一个缺陷(第 672 行)。2004 年 12 月 25 日是个周六。下一个周六是 2005 年 1 月 1 日。然而,运行测试时,会看到 getFollowingDayOfWeek 返回 12 月 25 日之后的周六还是 12 月 25 日。显然这不对[G3][t1]。我们看到问题在第 685 行。那是个典型的边界条件错误[T5]。应该是这样:
685 if (baseDOW >= targetWeekday) {
It is interesting to note that this function was the target of an earlier repair. The change history (line 43) shows that “bugs” were fixed in getPreviousDayOfWeek, getFollowingDayOfWeek, and getNearestDayOfWeek [T6].
很有意思,这个函数是之前一次修改的结果。修改记录(第 43 行)显示,getPreviousDayOfWeek、getFollowingDayOfWeek 和 getNearestDayOfWeek 中的“缺陷”已被修正[T6]。
The testGetNearestDayOfWeek unit test (line 329), which tests the getNearestDayOfWeek method (line 705), did not start out as long and exhaustive as it currently is. I added a lot of test cases to it because my initial test cases did not all pass [T6]. You can see the pattern of failure by looking at which test cases are commented out. That pattern is revealing [T7]. It shows that the algorithm fails if the nearest day is in the future. Clearly there is some kind of boundary condition error [T5].
测试 getNearestDayOfWeek(第 705 行)的单元测试 testGetNearestDayOfWeek(第 329 行)之前的版本不像现在一样没有遗漏。我添加了大量测试用例,因为初始的测试用例并没有全部通过 [T6]。查看哪些测试用例被注释掉,你可以看到失败的模式,这很有启发。如果最近的日期是在未来,算法就会失败。显然存在某种边界条件错误[T5]。
The pattern of test coverage reported by Clover is also interesting [T8]. Line 719 never gets executed! This means that the if statement in line 718 is always false. Sure enough, a look at the code shows that this must be true. The adjust variable is always negative and so cannot be greater or equal to 4. So this algorithm is just wrong.
Clover 汇报的测试覆盖模式也很有趣[T8]。第 719 行根本没有执行!这意味着第 718 行的 if 语句总是得到 false 的结果。没错,看一眼代码就知道是这样。变量 adjust 总是为负,所以不会大于或等于 4。所以,算法错了。
The right algorithm is shown below:
正确的算法如下所示:
int delta = targetDOW - base.getDayOfWeek();
int positiveDelta = delta + 7;
int adjust = positiveDelta % 7;
if (adjust > 3)
adjust -= 7;
return SerialDate.addDays(adjust, base);
2
3
4
5
6
7
Finally, the tests at line 417 and line 429 can be made to pass simply by throwing an IllegalArgumentException instead of returning an error string from weekInMonthToString and relativeToString.
最后,只要简单地抛出 IllegalArgumentException 异常而不是从 weekInMonthToString 和 relativeToString 返回错误字符串,第 417 行和 429 行的测试也能通过。
With these changes all the unit tests pass, and I believe SerialDate now works. So now it’s time to make it “right.”
做出这些修改后,所有的单元测试都通过了,我确信 SerialDate 现下可以工作。是时候让它“做对”了。
# 16.2 THEN MAKE IT RIGHT 让它做对
We are going to walk from the top to the bottom of SerialDate, improving it as we go along. Although you won’t see this in the discussion, I will be running all of the JCommon unit tests, including my improved unit test for SerialDate, after every change I make. So rest assured that every change you see here works for all of JCommon.
我们将从头到尾遍历 SerialDate,同时加以改进。尽管在本章的讨论中你看不到这个过程,在每次做修改后,我还是要运行全部 JCommon 单元测试,包括我为 SerialDate 改进的那些单元测试。所以,后面你看到的所有修改,对于 JCommon 都是可工作的。
Starting at line 1, we see a ream of comments with license information, copyrights, authors, and change history. I acknowledge that there are certain legalities that need to be addressed, and so the copyrights and licenses must stay. On the other hand, the change history is a leftover from the 1960s. We have source code control tools that do this for us now. This history should be deleted [C1].
从第 1 行开始,我看到大量有关许可、版权、作者和修改历史的注释。我明白,的确有些法律事宜要说明,所以版权和许可信息应该保留。另外,修改历史是产生于 19 世纪 60 年代的古董,现今源代码控制工具可以帮我们做到这个。应该删掉修改历史[C1]。
The import list starting at line 61 could be shortened by using java.text.*
and java.util.*
. [J1]
从第 61 行开始的导入列表应该通过使用
java.text.*
和java.util.*
来缩短。[J1]
I wince at the HTML formatting in the Javadoc (line 67). Having a source file with more than one language in it troubles me. This comment has four languages in it: Java, English, Javadoc, and html [G1]. With that many languages in use, it’s hard to keep things straight. For example, the nice positioning of line 71 and line 72 are lost when the Javadoc is generated, and yet who wants to see <ul>
and <li>
in the source code? A better strategy might be to just surround the whole comment with <pre>
so that the formatting that is apparent in the source code is preserved within the Javadoc.1
Javadoc 的 HTML 格式化工作(第 67 行)令我畏惧。一个源文件里面有多种语言,我有点发怵。这条注释有 4 种语言:Java、英文、Javadoc 和 html[G1]。有那么多语言,就很难直截了当。例如,生成 Javadoc 后,第 71 行和 72 行原本很好的位置就丢失了,而且谁想在源代码中看到
<ul>
和<li>
这样的东西呢?更好的策略可能是用<pre>
标签把整个注释部分包围起来,这样,对于源代码的格式化只会限于 Javadoc 之内[1]。
Line 86 is the class declaration. Why is this class named SerialDate? What is the significance of the word “serial”? Is it because the class is derived from Serializable? That doesn’t seem likely.
第 86 行是类声明。这个类为何要命名为 SerialDate?Serial 一词有什么妙处吗?是不是因为该类派生自 Serializable?看来不是这样的。
I won’t keep you guessing. I know why (or at least I think I know why) the word “serial” was used. The clue is in the constants SERIAL_LOWER_BOUND and SERIAL_UPPER_BOUND on line 98 and line 101. An even better clue is in the comment that begins on line 830. This class is named SerialDate because it is implemented using a “serial number,” which happens to be the number of days since December 30th, 1899.
别猜了,我知道为什么(或者我认为自己知道)何以要用 Serial 一词。线索就在位于第 98 行和 101 行的常量 SERIAL LOWER BOUND 和 SERIAL UPPER BOUND。更好的线索在从第 830 行开始的注释中。该类被命名为 SerialDate,是因为它用“序列数”(serial number)来实现,该系列数恰好是从 1899 年 12 月 30 日后的天数。
I have two problems with this. First, the term “serial number” is not really correct. This may be a quibble, but the representation is more of a relative offset than a serial number. The term “serial number” has more to do with product identification markers than dates. So I don’t find this name particularly descriptive [N1]. A more descriptive term might be “ordinal.”
对此我有两个问题。首先,术语“序列数”并不真对。可能有点诡辩,但其呈现方式却更接近相对偏移甚于序列数。术语“序列数”更多地用于产品版本标识,而非日期标识。我没发现这个名称特别有描述力[N1]。更有描述力的术语大概是“顺序”(ordinal)。
The second problem is more significant. The name SerialDate implies an implementation. This class is an abstract class. There is no need to imply anything at all about the implementation. Indeed, there is good reason to hide the implementation! So I find this name to be at the wrong level of abstraction [N2]. In my opinion, the name of this class should simply be Date.
第二个问题更突出。名称 SerialDate 暗示了一种实现。该类是个抽象类。没必要暗示任何有关实现的事。实际上,没理由隐藏实现!我发现这个名称放在了不正确的抽象层级上[N2]。以我之见,该类的名称应该就是简单的 Date。
Unfortunately, there are already too many classes in the Java library named Date, so this is probably not the best name to choose. Because this class is all about days, instead of time, I considered naming it Day, but this name is also heavily used in other places. In the end, I chose DayDate as the best compromise.
不幸的是,Java 类库里面有太多叫 Date 的类了,所以这大概也不是最好的名称。因为这个类是关于日期而非时间,我想将其命名为 Day,但这个名字也在多处被滥用。最后,我选了 DayDate 作为最佳折衷方案。
From now on in this discussion I will use the term DayDate. I leave it to you to remember that the listings you are looking at still use SerialDate.
从现在起,我将使用术语 DayDate。请记住,你读到的代码清单,还是用的 SerialDate。
I understand why DayDate inherits from Comparable and Serializable. But why does it inherit from MonthConstants? The class MonthConstants (Listing B-3, page 372) is just a bunch of static final constants that define the months. Inheriting from classes with constants is an old trick that Java programmers used so that they could avoid using expressions like MonthConstants.January, but it’s a bad idea [J2]. MonthConstants should really be an enum.
我理解为何 DayDate 继承自 Comparable 和 Serializable。不过,为什么它要继承自 MonthConstants 呢?类 MonthConstants(见代码清单 B-3)只是一大堆定义了月份的静态常量。从常量类继承是 Java 程序员用的一种老花招,这样他们就能避免形如 MonthConstants.January 的表达式,不过这是个坏主意[J2]。MonthConstants 其实应该是个枚举。
public abstract class DayDate implements Comparable, Serializable {
public static enum Month {
JANUARY(1),
FEBRUARY(2),
MARCH(3),
APRIL(4),
MAY(5),
JUNE(6),
JULY(7),
AUGUST(8),
SEPTEMBER(9),
OCTOBER(10),
NOVEMBER(11),
DECEMBER(12);
Month(int index) {
this.index = index;
}
public static Month make(int monthIndex) {
for (Month m : Month.values()) {
if (m.index == monthIndex)
return m;
}
throw new IllegalArgumentException("Invalid month index " + monthIndex);
}
public final int index;
}
}
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
Changing MonthConstants to this enum forces quite a few changes to the DayDate class and all it’s users. It took me an hour to make all the changes. However, any function that used to take an int for a month, now takes a Month enumerator. This means we can get rid of the isValidMonthCode method (line 326), and all the month code error checking such as that in monthCodeToQuarter (line 356) [G5].
把 MonthConstants 改成枚举,导致对 DayDate 类和用到这个类的代码的一些修改。我花了一个小时来改代码。不过,原来以 int 为月份类型的函数,现在都用上 Month 枚举元素了。这意味着我们可以去除 isValidMonthCode 方法(第 326 行),以及 monthCodeToQuarter 等位置的月份代码错误检查(第 356 行)了[G5]。
Next, we have line 91, serialVersionUID. This variable is used to control the serializer. If we change it, then any DayDate written with an older version of the software won’t be readable anymore and will result in an InvalidClassException. If you don’t declare the serialVersionUID variable, then the compiler automatically generates one for you, and it will be different every time you make a change to the module. I know that all the documents recommend manual control of this variable, but it seems to me that automatic control of serialization is a lot safer [G4]. After all, I’d much rather debug an InvalidClassException than the odd behavior that would ensue if I forgot to change the serialVersionUID. So I’m going to delete the variable—at least for the time being.2
下一步,我们看到第 91 行,serialVersionUID。该变量用于控制序列号。如果我们修改了它,用这个软件编写的旧版本 DayDate 都将不再可用,而是返回一个 InvalidClassException 异常。如果你没有声明 serialVersionUID 变量,则编译器会自动生成一个,每次修改模块时都会得到不一样的值。我知道,所有的文档都建议手工控制这个变量,但对我来说自动控制序列号安全得多[G4]。我宁肯调试 InvalidClassException,也不愿意见到如果忘记修改 serialVersionUID 引起的后续工作。所以,我要删除这个变量——至少暂时这么做[2]。
I find the comment on line 93 redundant. Redundant comments are just places to collect lies and misinformation [C2]. So I’m going to get rid of it and its ilk.
我发现第 93 行的注释是多余的。这正是谎言和误导信息所在之地[C2]。所以我要干掉它和它的同类。
The comments at line 97 and line 100 talk about serial numbers, which I discussed earlier [C1]. The variables they describe are the earliest and latest possible dates that DayDate can describe. This can be made a bit clearer [N1].
第 97 行和 100 行的注释有关序列数,我之前已经讨论过这个问题[C1]。它们描述的变量是 DayDate 能够描述的最早和最后的日期。这可以搞得更清楚些[N1]。
public static final int EARLIEST_DATE_ORDINAL = 2; // 1/1/1900
public static final int LATEST_DATE_ORDINAL = 2958465; // 12/31/9999
2
It’s not clear to me why EARLIEST_DATE_ORDINAL is 2 instead of 0. There is a hint in the comment on line 829 that suggests that this has something to do with the way dates are represented in Microsoft Excel. There is a much deeper insight provided in a derivative of DayDate called SpreadsheetDate (Listing B-5, page 382). The comment on line 71 describes the issue nicely.
我不太清楚为什么 EARLIEST_DATE_ORDINAL 是 2 而不是 0。在第 829 行的注释中有个提示,说明这与用 Microsoft Excel 展示日期的方式有关。在 DayDate 的派生类 SpredsheetDate 中能看得更深入(见代码清单 B-5)。第 71 行的注释很好地描述了这个问题。
The problem I have with this is that the issue seems to be related to the implementation of SpreadsheetDate and has nothing to do with DayDate. I conclude from this that EARLIEST_DATE_ORDINAL and LATEST_DATE_ORDINAL do not really belong in DayDate and should be moved to SpreadsheetDate [G6].
我的问题是,这看来应该与 SpreadsheetDate 有关,与 DayDate 无关才对。所以,EARLIEST_DATE_ORDINAL 和 LATEST_DATE_ORDINAL 实在不该属于 DayDate,应该移到 SpreadSheeDate 中[G6]。
Indeed, a search of the code shows that these variables are used only within SpreadsheetDate. Nothing in DayDate, nor in any other class in the JCommon framework, uses them. Therefore, I’ll move them down into SpreadsheetDate.
的确,搜索一下代码就知道,这些变量值仅在 SpreadSheetDate 中用到。DayDate 中没用到,JCommon 框架的其他类中也没有用。所以,我将把它们向下移到 SpreadSheetDate 中。
The next variables, MINIMUM_YEAR_SUPPORTED, and MAXIMUM_YEAR_SUPPORTED (line 104 and line 107), provide something of a dilemma. It seems clear that if DayDate is an abstract class that provides no foreshadowing of implementation, then it should not inform us about a minimum or maximum year. Again, I am tempted to move these variables down into SpreadsheetDate [G6]. However, a quick search of the users of these variables shows that one other class uses them: RelativeDayOfWeekRule (Listing B-6, page 390). We see that usage at line 177 and line 178 in the getDate function, where they are used to check that the argument to getDate is a valid year. The dilemma is that a user of an abstract class needs information about its implementation.
下面两个变量,MINIMUN_YEAR_SUPPORTED 和 MAXIMUM_YEAR_SUPPORTED(第 104 行和 107 行)地位尴尬。显然,如果 DayDate 是个没有提供实现铺垫的抽象类,它就不该告知我们有关最小和最大年份的信息。同样,我很想把这些变量向下移到 SpreadSheetDate 中[G6]。然而,快速查找这些变量的使用情况,会发现另一个类也在用:RelativeDayOfWeekRule (见代码清单 B-6)。在第 177 行和 178 行,getDate 函数中,它们被用来检查 getDate 的年份参数是否有效。抽象类的用户需要得知其实现信息,这是个矛盾。
What we need to do is provide this information without polluting DayDate itself. Usually, we would get implementation information from an instance of a derivative. However, the getDate function is not passed an instance of a DayDate. It does, however, return such an instance, which means that somewhere it must be creating it. Line 187 through line 205 provide the hint. The DayDate instance is being created by one of the three functions, getPreviousDayOfWeek, getNearestDayOfWeek, or getFollowingDayOfWeek. Looking back at the DayDate listing, we see that these functions (lines 638–724) all return a date created by addDays (line 571), which calls createInstance (line 808), which creates a SpreadsheetDate! [G7].
我们要做的是既提供信息,又不污染 DayDate。通常,我们会从派生类实体中获取实现信息。不过,并未向 getDate 函数传入 DayDate 的实体,反而返回了这么一个实体。这意味着必须在某处创建实体。第 187 ~ 205 行提供了线索。DayDate 实体是在 getPreviousDayOfWeek、getNearestDayOfWeek 或 getFollowingDayOfWeek 这三个函数其中之一里面创建的。看回 DayDate 代码清单,我们看到,这些函数(第 638 ~ 724 行)全都返回了由 addDays(第 571 行)创建的日期实体,addDays 调用 CreateInstance(第 808 行),创建出一个 SpreadSheetDate![G7]。
It’s generally a bad idea for base classes to know about their derivatives. To fix this, we should use the ABSTRACT FACTORY3 pattern and create a DayDateFactory. This factory will create the instances of DayDate that we need and can also answer questions about the implementation, such as the maximum and minimum dates.
通常来说,基类不宜了解其派生类的情况。为了修正这个毛病,我们应该利用抽象工厂模式(ABSTRACT FACTORY)[3],创建一个 DayDateFactory。该工厂将创建我们所需要的 DayDate 的实体,并回答有关实现的问题,例如最大和最小日期之类。
public abstract class DayDateFactory {
private static DayDateFactory factory = new SpreadsheetDateFactory();
public static void setInstance(DayDateFactory factory) {
DayDateFactory.factory = factory;
}
protected abstract DayDate _makeDate(int ordinal);
protected abstract DayDate _makeDate(int day, DayDate.Month month, int year);
protected abstract DayDate _makeDate(int day, int month, int year);
protected abstract DayDate _makeDate(java.util.Date date);
protected abstract int _getMinimumYear();
protected abstract int _getMaximumYear();
public static DayDate makeDate(int ordinal) {
return factory._makeDate(ordinal);
}
public static DayDate makeDate(int day, DayDate.Month month, int year) {
return factory._makeDate(day, month, year);
}
public static DayDate makeDate(int day, int month, int year) {
return factory._makeDate(day, month, year);
}
public static DayDate makeDate(java.util.Date date) {
return factory._makeDate(date);
}
public static int getMinimumYear() {
return factory._getMinimumYear();
}
public static int getMaximumYear() {
return factory._getMaximumYear();
}
}
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
This factory class replaces the createInstance methods with makeDate methods, which improves the names quite a bit [N1]. It defaults to a SpreadsheetDateFactory but can be changed at any time to use a different factory. The static methods that delegate to abstract methods use a combination of the SINGLETON,4 DECORATOR,5 and ABSTRACT FACTORY patterns that I have found to be useful.
该工厂类用 makeDate 方法替代了 createInstance 方法,前者的名称稍好一些[N1]。在初始状态下,它使用 SpreadsheetDateFactory,但随时可以使用其他工厂。委托到抽象方法的静态方法混合采用了单件模式(SINGLETON)、油漆工模式[4]和抽象工厂模式[5],我发现这种手段很有用。
The SpreadsheetDateFactory looks like this.
SpreadsheetDateFactory 看起来像这个样子:
public class SpreadsheetDateFactory extends DayDateFactory {
public DayDate _makeDate(int ordinal) {
return new SpreadsheetDate(ordinal);
}
public DayDate _makeDate(int day, DayDate.Month month, int year) {
return new SpreadsheetDate(day, month, year);
}
public DayDate _makeDate(int day, int month, int year) {
return new SpreadsheetDate(day, month, year);
}
public DayDate _makeDate(Date date) {
final GregorianCalendar calendar = new GregorianCalendar();
calendar.setTime(date);
return new SpreadsheetDate(
calendar.get(Calendar.DATE),
DayDate.Month.make(calendar.get(Calendar.MONTH) + 1),
calendar.get(Calendar.YEAR));
}
protected int _getMinimumYear() {
return SpreadsheetDate.MINIMUM_YEAR_SUPPORTED;
}
protected int _getMaximumYear() {
return SpreadsheetDate.MAXIMUM_YEAR_SUPPORTED;
}
}
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
As you can see, I have already moved the MINIMUM_YEAR_SUPPORTED and MAXIMUM_YEAR_SUPPORTED variables into SpreadsheetDate, where they belong [G6].
如你所见,我已经把 MINIMUM_YEAR_SUPPORTED 和 MAXIMUM_YEAR_SUPPORTED 变量移到了它们该在的 SpreadsheetDate 中[G6]。
The next issue in DayDate are the day constants beginning at line 109. These should really be another enum [J3]. We’ve seen this pattern before, so I won’t repeat it here. You’ll see it in the final listings.
DayDate 的下一个问题是第 109 行的日期常量。这些常量其实应该是枚举[J3]。我们之前见过这种模式,不再赘述。你可以在最终的代码清单中看到。
Next, we see a series of tables starting with LAST_DAY_OF_MONTH at line 140. My first issue with these tables is that the comments that describe them are redundant [C3]. Their names are sufficient. So I’m going to delete the comments.
跟着,我们看到第 140 行一系列以 LAST_DAY_OF_MONTH 开头的数组。首先,描述这些数组的注释全属多余[C3]。光看名称就够了。所以我要删除这些注释。
There seems to be no good reason that this table isn’t private [G8], because there is a static function lastDayOfMonth that provides the same data.
这个数组没理由不是私有的[G8],因为有个静态函数 lastDayOfMonth 提供同样的数据。
The next table, AGGREGATE_DAYS_TO_END_OF_MONTH, is a bit more mysterious because it is not used anywhere in the JCommon framework [G9]. So I deleted it.
下一个数组 AGGREGATE_DAYS_TO_END_OF_MONTH 更神秘一些,在 JCommon 框架中根本没用到它[G9]。所以我直接删除了。
The same goes for LEAP_YEAR_AGGREGATE_DAYS_TO_END_OF_MONTH.
对于 LEAP_YEAR_AGGREGATE_DAYS_TO_END_OF_MONTH 也一样。
The next table, AGGREGATE_DAYS_TO_END_OF_PRECEDING_MONTH, is used only in Spread-sheetDate (line 434 and line 473). This begs the question of whether it should be moved to SpreadsheetDate. The argument for not moving it is that the table is not specific to any particular implementation [G6]. On the other hand, no implementation other than SpreadsheetDate actually exists, and so the table should be moved close to where it is used [G10].
AGGREGATE_DAYS_TO_END_OF_PRECEDING_MONTH 只在 SpreadsheetDate 中用到(第 434 行和 473 行)。是否把它移到 SpreadsheetDate 中去是个问题。不转移的理由是,该数组并不专属于任何特定的实现[G6]。另一方面,实际上并不存在 SpreadsheetDate 之外的实现,所以,数组应该移到靠近其使用位置的地方[G10]。
What settles the argument for me is that to be consistent [G11], we should make the table private and expose it through a function like julianDateOfLastDayOfMonth. Nobody seems to need a function like that. Moreover, the table can be moved back to DayDate easily if any new implementation of DayDate needs it. So I moved it.
说服我的理由是保持一致[G11],数组应该私有,并通过类似 julianDateOfLastDayOfMonth 这样的函数来暴露。看来没人需要那样的函数。而且,如果有新的 DayDate 实现需要该数组,可以轻易地把它移回到 DayDate 中去。所以我就把它移到 SpreadsheetDate 里面了。
The same goes for the table, LEAP_YEAR_AGGREGATE_DAYS_TO_END_OF_MONTH.
对于 LEAP_YEAR_AGGREGATE_DAYS_TO_END_OF_MONTH.也一样。
Next, we see three sets of constants that can be turned into enums (lines 162–205). The first of the three selects a week within a month. I changed it into an enum named WeekInMonth.
跟着,我们看到三组可以转换为枚举的常量(第 162 ~ 205 行)。第一个用来选择月份中的一周。我将其转换为名为 WeekInMonth 的枚举。
public enum WeekInMonth {
FIRST(1), SECOND(2), THIRD(3), FOURTH(4), LAST(0);
public final int index;
WeekInMonth(int index) {
this.index = index;
}
}
2
3
4
5
6
7
8
The second set of constants (lines 177–187) is a bit more obscure. The INCLUDE_NONE, INCLUDE_FIRST, INCLUDE_SECOND, and INCLUDE_BOTH constants are used to describe whether the defining end-point dates of a range should be included in that range. Mathematically, this is described using the terms “open interval,” “half-open interval,” and “closed interval.” I think it is clearer using the mathematical nomenclature [N3], so I changed it to an enum named DateInterval with CLOSED, CLOSED_LEFT, CLOSED_RIGHT, and OPEN enumerators.
第二组常量(第 177 ~ 187 行)有点麻烦。INCLUDE_NONE、INCLUDE_FIRST、INCLUDE_SECOND 和 INCLUDE_BOTH 常量用于描述某个范围的终止日是否包含在该范围之内。数学上,用术语“开放区间”、“半开放区间”和“闭合区间”来表示。我想,用数学术语来命名会更清晰[N3],所以就将其转换为枚举 DateInterval,其中包括 CLOSED、CLOSED_LEFT、CLOSED_RIGHT 和 OPEN 枚举元素。
The third set of constants (lines 189–205) describe whether a search for a particular day of the week should result in the last, next, or nearest instance. Deciding what to call this is difficult at best. In the end, I settled for WeekdayRange with LAST, NEXT, and NEAREST enumerators.
第三组常量(第 18 ~ 205 行)描述了是否该在最后、下一个或最近的日期实体中呈现对某个星期的特定一天的查找结果。怎么命名是个难题。最终,我给 WeekdayRange 设定了 LAST、NEXT 和 NEAREST 枚举元素。
You might not agree with the names I’ve chosen. They make sense to me, but they may not make sense to you. The point is that they are now in a form that makes them easy to change [J3]. They aren’t passed as integers anymore; they are passed as symbols. I can use the “change name” function of my IDE to change the names, or the types, without worrying that I missed some -1 or 2 somewhere in the code or that some int argument declaration is left poorly described.
你也许不会同意我取的名字。对我而言这些名字有意义,但对你可能就不然。要点是它们眼下变成了易于修改的形式[J3]。不再以整数形式传递,而是作为符号传递。我可以用 IDE 的“修改名称”功能来改动名称或类型,无需担忧漏掉代码中某处-1 或 2 之类的数字,也不必担忧某些 int 参数声明处于描述不佳的状态。
The description field at line 208 does not seem to be used by anyone. I deleted it along with its accessor and mutator [G9].
第 208 行的描述字段看来没有任何地方用到。我把它及其取值器和赋值器都删掉了。
I also deleted the degenerate default constructor at line 213 [G12]. The compiler will generate it for us.
我还删除了第 213 行的默认构造器[G12]。编译器会为我们自动生成的。
We can skip over the isValidWeekdayCode method (lines 216–238) because we deleted it when we created the Day enumeration.
略过 isValidWeekdayCode 方法(第 216 ~ 238 行),在创建 Day 枚举时已经把它删掉了。
This brings us to the stringToWeekdayCode method (lines 242–270). Javadocs that don’t add much to the method signature are just clutter [C3],[G12]. The only value this Javadoc adds is the description of the -1 return value. However, because we changed to the Day enumeration, the comment is actually wrong [C2]. The method now throws an IllegalArgumentException. So I deleted the Javadoc.
于是来到 stringToWeekdayCode 方法(第 242 ~ 270 行)。没有方法签名增添价值的 Javadoc 都是废话[C3]、[G12],唯一的价值是对返回值-1 的描述。然而,因为我们改用了 Day 枚举,这条注释就完全错误了[C2]。该方法现在抛出一个 IllegalArgumentException 异常。所以我删除了 Javadoc。
I also deleted all the final keywords in arguments and variable declarations. As far as I could tell, they added no real value but did add to the clutter [G12]. Eliminating final flies in the face of some conventional wisdom. For example, Robert Simmons6 strongly recommends us to “. . . spread final all over your code.” Clearly I disagree. I think that there are a few good uses for final, such as the occasional final constant, but otherwise the keyword adds little value and creates a lot of clutter. Perhaps I feel this way because the kinds of errors that final might catch are already caught by the unit tests I write.
我还删除了参数和变量声明中的全部 final 关键字。我敢说,它们毫无价值,空自混淆视听惑[G12]。删除这些 final,不合某些成例。例如,Robert Simmons[6]就强烈建议我们“……在代码中遍布 final。”我不能苟同。我认为,final 有少数的好用法,例如偶尔使用的 final 常量,但除此之外该关键字利小于弊。我这么认为,或许是因为 final 可能捕获到的那些错误类型,早已被我编写的单元测试捕获了。
I didn’t care for the duplicate if statements [G5] inside the for loop (line 259 and line 263), so I connected them into a single if statement using the || operator. I also used the Day enumeration to direct the for loop and made a few other cosmetic changes.
我不喜欢 for 循环(第 259 行和 263 行)中的那些 if 语句[G5],所以我利用“||”操作符把它们连接为单个 if 语句。我还使用 Day 枚举整理 for 循环,做了一些装饰性的修改。
It occurred to me that this method does not really belong in DayDate. It’s really the parse function of Day. So I moved it into the Day enumeration. However, that made the Day enumeration pretty large. Because the concept of Day does not depend on DayDate, I moved the Day enumeration outside of the DayDate class into its own source file [G13].
我认为,这个方法并不真属于 DayDate 类。它其实是 Day 的一个解析函数。所以,我将它移到 Day 枚举中。不过,那样 Day 枚举就会变得太大。因为 Day 的概念并不依赖于 DayDate,我就把 Day 枚举移到 DayDate 类之外,放到它自己的源代码文件中。
I also moved the next function, weekdayCodeToString (lines 272–286) into the Day enumeration and called it toString.
我还把下一个函数,weekdayCodeToString(第 272 ~ 286 行),移植到 Day 枚举中,称其为 toString。
public enum Day {
MONDAY(Calendar.MONDAY),
TUESDAY(Calendar.TUESDAY),
WEDNESDAY(Calendar.WEDNESDAY),
THURSDAY(Calendar.THURSDAY),
FRIDAY(Calendar.FRIDAY),
SATURDAY(Calendar.SATURDAY),
SUNDAY(Calendar.SUNDAY);
public final int index;
private static DateFormatSymbols dateSymbols = new DateFormatSymbols();
Day(int day) {
index = day;
}
public static Day make(int index) throws IllegalArgumentException {
for (Day d : Day.values())
if (d.index == index)
return d;
throw new IllegalArgumentException(
String.format("Illegal day index: %d.", index));
}
public static Day parse(String s) throws IllegalArgumentException {
String[] shortWeekdayNames =
dateSymbols.getShortWeekdays();
String[] weekDayNames =
dateSymbols.getWeekdays();
s = s.trim();
for (Day day : Day.values()) {
if (s.equalsIgnoreCase(shortWeekdayNames[day.index]) ||
s.equalsIgnoreCase(weekDayNames[day.index])) {
return day;
}
}
throw new IllegalArgumentException(
String.format("%s is not a valid weekday string", s));
}
public String toString() {
return dateSymbols.getWeekdays()[index];
}
}
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
There are two getMonths functions (lines 288–316). The first calls the second. The second is never called by anyone but the first. Therefore, I collapsed the two into one and vastly simplified them [G9],[G12],[F4]. Finally, I changed the name to be a bit more self-descriptive [N1].
有两个 getMonth 函数(第 288 ~ 316 行)。第一个函数调用第二个函数。第二个函数只被第一个函数调用。所以,我把这两个函数合二为一,而且极大地简化之[G9][g12][F4]。最后,我把名称修改得更具自我描述力[N1]。
public static String[] getMonthNames() {
return dateFormatSymbols.getMonths();
}
2
3
The isValidMonthCode function (lines 326–346) was made irrelevant by the Month enum, so I deleted it [G9].
由于有了 Month 枚举,函数 isValidMonthCode(第 326 ~ 346 行)就变得没什么用,所以我把它删除了[G9]。
The monthCodeToQuarter function (lines 356–375) smells of FEATURE ENVY7 [G14] and probably belongs in the Month enum as a method named quarter. So I replaced it.
函数 monthCodeToQuarter(第 356 ~ 375 行)有特性依恋(FEATURE ENVY)[7]的味道,可以是 Month 枚举中的一个名为 quarter 的方法,我就这么办了。
- [Refactoring].
public int quarter() {
return 1 + (index-1)/3;
}
2
3
This made the Month enum big enough to be in its own class. So I moved it out of DayDate to be consistent with the Day enum [G11],[G13].
这样一来,Month 枚举就大到需要放到自己的类中了。我把它从 DayDate 中移出来,与 Day 枚举保持一致[G11][g13]。
The next two methods are named monthCodeToString (lines 377–426). Again, we see the pattern of one method calling its twin with a flag. It is usually a bad idea to pass a flag as an argument to a function, especially when that flag simply selects the format of the output [G15]. I renamed, simplified, and restructured these functions and moved them into the Month enum [N1],[N3],[C3],[G14].
下两个方法被命名为 monthCodeToString(第 377 ~ 426 行)。我们再次看到其中一个方法使用标识调用其兄弟方法的模式。将标识作为参数传递给函数的做法通常不太好,尤其是当该标识只是有关其输出格式时[G15]。我重命名、简化、重新构架了这些函数,并把它们移到 Month 枚举中[N1][n3][G14]。
public String toString() {
return dateFormatSymbols.getMonths()[index - 1];
}
public String toShortString() {
return dateFormatSymbols.getShortMonths()[index - 1];
}
2
3
4
5
6
7
The next method is stringToMonthCode (lines 428–472). I renamed it, moved it into the Month enum, and simplified it [N1],[N3],[C3],[G14],[G12].
下一个方法是 stringToMonthCode(第 428 ~ 472 行)。我重新为它命名,转移到 Month 枚举中,并且简化之[N1][n3][C3][g14][G12]。
public static Month parse(String s) {
s = s.trim();
for (Month m : Month.values())
if (m.matches(s))
return m;
try {
return make(Integer.parseInt(s));
}
catch (NumberFormatException e) {}
throw new IllegalArgumentException(“Invalid month ” + s);
}
private boolean matches(String s) {
return s.equalsIgnoreCase(toString()) ||
s.equalsIgnoreCase(toShortString());
}
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
The isLeapYear method (lines 495–517) can be made a bit more expressive [G16].
方法 isLeapYear(第 495 ~ 517 行)可以写得更具表达力一些[G16]。
public static boolean isLeapYear(int year) {
boolean fourth = year % 4 == 0;
boolean hundredth = year % 100 == 0;
boolean fourHundredth = year % 400 == 0;
return fourth && (!hundredth || fourHundredth);
}
2
3
4
5
6
The next function, leapYearCount (lines 519–536) doesn’t really belong in DayDate. Nobody calls it except for two methods in SpreadsheetDate. So I pushed it down [G6].
下一个函数 leapYearCount(第 519 ~ 536 行)并不真属于 DayDate。除了 SpreadsheetDate 中的两个方法外,没有其他调用者。所以我将它往下放。
The lastDayOfMonth function (lines 538–560) makes use of the LAST_DAY_OF_MONTH array. This array really belongs in the Month enum [G17], so I moved it there. I also simplified the function and made it a bit more expressive [G16].
函数 lastDayOfMonth(第 538 ~ 560 行)使用了 LAST_DAY_OF_MONTH 数组。该数组应该隶属于 Month 枚举[G17],所以我就把它移到那儿去了。我还简化了这个函数,使其更具表达力[G16]。
public static int lastDayOfMonth(Month month, int year) {
if (month == Month.FEBRUARY && isLeapYear(year))
return month.lastDay() + 1;
else
return month.lastDay();
}
2
3
4
5
6
Now things start to get a bit more interesting. The next function is addDays (lines 562–576). First of all, because this function operates on the variables of DayDate, it should not be static [G18]. So I changed it to an instance method. Second, it calls the function toSerial. This function should be renamed toOrdinal [N1]. Finally, the method can be simplified.
现在,事情变得比较有趣一些了。下一个函数是 addDays(第 562 ~ 576 行)。首先,由于该函数对 DayDate 的变量进行操作,它就不该是静态的[G18]。所以,我把它修改为实体方法。其次,它调用了函数 toSerial。这个函数应该重新命名为 toOrdial [N1]。最后,该方法可以简化。
public DayDate addDays(int days) {
return DayDateFactory.makeDate(toOrdinal() + days);
}
2
3
The same goes for addMonths (lines 578–602). It should be an instance method [G18]. The algorithm is a bit complicated, so I used EXPLAINING TEMPORARY VARIABLES8 [G19] to make it more transparent. I also renamed the method getYYY to getYear [N1].
对于 addMonth(第 578 ~ 602 行)也一样。它应该是个实体方法[G18]。算法太过复杂,所以我利用解释临时变量模式(EXPLAINING TEMPORARY VARIABLES)[8]来使其更为透明。我还将方法 getYYY 重命名为 getYear [N1]。
public DayDate addMonths(int months) {
int thisMonthAsOrdinal = 12 * getYear() + getMonth().index - 1;
int resultMonthAsOrdinal = thisMonthAsOrdinal + months;
int resultYear = resultMonthAsOrdinal / 12;
Month resultMonth = Month.make(resultMonthAsOrdinal % 12 + 1);
int lastDayOfResultMonth = lastDayOfMonth(resultMonth, resultYear);
int resultDay = Math.min(getDayOfMonth(), lastDayOfResultMonth);
return DayDateFactory.makeDate(resultDay, resultMonth, resultYear);
}
2
3
4
5
6
7
8
9
10
The addYears function (lines 604–626) provides no surprises over the others.
对于函数 addYear(第 604 ~ 626 行)也照方办理。
public DayDate plusYears(int years) {
int resultYear = getYear() + years;
int lastDayOfMonthInResultYear = lastDayOfMonth(getMonth(), resultYear);
int resultDay = Math.min(getDayOfMonth(), lastDayOfMonthInResultYear);
return DayDateFactory.makeDate(resultDay, getMonth(), resultYear);
}
2
3
4
5
6
There is a little itch at the back of my mind that is bothering me about changing these methods from static to instance. Does the expression date.addDays(5) make it clear that the date object does not change and that a new instance of DayDate is returned? Or does it erroneously imply that we are adding five days to the date object? You might not think that is a big problem, but a bit of code that looks like the following can be very deceiving [G20].
把这些方法从静态方法变为实体方法,让我有点心头发痒。用 date.addDays(5)这样的表达方法,是不是明确地表示了 date 对象并没变动以及返回了一个 DayDate 的新实体呢?或者,它只是错误地暗示我们往 date 对象添加了 5 天呢?你可能不会认为这是个大问题,但下列代码却可能会有欺骗性。
DayDate date = DateFactory.makeDate(5, Month.DECEMBER, 1952);
date.addDays(7); // bump date by one week.
2
Someone reading this code would very likely just accept that addDays is changing the date object. So we need a name that breaks this ambiguity [N4]. So I changed the names to plusDays and plusMonths. It seems to me that the intent of the method is captured nicely by
有些读到这段代码的人会认为 addDays 在修改 date 对象。所以,我们需要消除这种歧义的名称[N4]。我把名称改为 plusDays 和 plusMonths。我认为,方法的初衷很清楚地被
DayDate date = oldDate.plusDays(5);
whereas the following doesn’t read fluidly enough for a reader to simply accept that the date object is changed:
所体现,不过下列代码对认为 date 对象被修改的读者来说,看起来并不那么顺畅:
date.plusDays(5);
The algorithms continue to get more interesting. getPreviousDayOfWeek (lines 628–660) works but is a bit complicated. After some thought about what was really going on [G21], I was able to simplify it and use EXPLAINING TEMPORARY VARIABLES [G19] to make it clearer. I also changed it from a static method to an instance method [G18], and got rid of the duplicate instance method [G5] (lines 997–1008).
算法越来越有趣,getPreviousDayOfWeek(第 628 ~ 660 行)可以工作,不过有点复杂了。经过一番思考,了解到它的功能后[G21],我就能够使用解释临时变量模式来简化它[G19],使其更为清晰。我还将它从静态方法改为实体方法[G18],并删除了重复的实体方法[G5](第 997 ~ 1008 行)。
public DayDate getPreviousDayOfWeek(Day targetDayOfWeek) {
int offsetToTarget = targetDayOfWeek.index - getDayOfWeek().index;
if (offsetToTarget >= 0)
offsetToTarget -= 7;
return plusDays(offsetToTarget);
}
2
3
4
5
6
The exact same analysis and result occurred for getFollowingDayOfWeek (lines 662–693).
对 getFollowingDayOfWeek(第 662 ~ 693 行)也如法炮制:
public DayDate getFollowingDayOfWeek(Day targetDayOfWeek) {
int offsetToTarget = targetDayOfWeek.index - getDayOfWeek().index;
if (offsetToTarget <= 0)
offsetToTarget += 7;
return plusDays(offsetToTarget);
}
2
3
4
5
6
7
The next function is getNearestDayOfWeek (lines 695–726), which we corrected back on page 270. But the changes I made back then aren’t consistent with the current pattern in the last two functions [G11]. So I made it consistent and used some EXPLAINING TEMPORARY VARIABLES [G19] to clarify the algorithm.
下一个函数是我们之前修改过的 getNearestDayOfWeek(第 695 ~ 726 行)。我之前所做的修改和前两个函数没有保持一致[G11]。所以我将它改得和这两个函数保持一致,并且使用解释临时变量模式[G19]来阐明算法。
public DayDate getNearestDayOfWeek(final Day targetDay) {
int offsetToThisWeeksTarget = targetDay.index - getDayOfWeek().index;
int offsetToFutureTarget = (offsetToThisWeeksTarget + 7) % 7;
int offsetToPreviousTarget = offsetToFutureTarget - 7;
if (offsetToFutureTarget > 3)
return plusDays(offsetToPreviousTarget);
else
return plusDays(offsetToFutureTarget);
}
2
3
4
5
6
7
8
9
10
The getEndOfCurrentMonth method (lines 728–740) is a little strange because it is an instance method that envies [G14] its own class by taking a DayDate argument. I made it a true instance method and clarified a few names.
方法 getEndOfCurrentMonth(第 728 ~ 740 行)有点奇怪,因为它获取了 DayDate 参数,从而成为一个依恋[G14]其自身类的实体方法。我将其改为真正的实体方法,并修改了几个名称。
public DayDate getEndOfMonth() {
Month month = getMonth();
int year = getYear();
int lastDay = lastDayOfMonth(month, year);
return DayDateFactory.makeDate(lastDay, month, year);
}
2
3
4
5
6
Refactoring weekInMonthToString (lines 742–761) turned out to be very interesting indeed. Using the refactoring tools of my IDE, I first moved the method to the WeekInMonth enum that I created back on page 275. Then I renamed the method to toString. Next, I changed it from a static method to an instance method. All the tests still passed. (Can you guess where I am going?)
重构 weekInMonthToString(第 742 ~ 761 行)的过程非常有趣。利用 IDE 的重构工具,我先将其移到我之前创建的 WeekInMonth 枚举中,再将其重命名为 toString。跟着,我把它从静态方法改为实体方法。所有的测试都通过了。(你能猜出来我打算做什么吗?)
Next, I deleted the method entirely! Five asserts failed (lines 411–415, Listing B-4, page 374). I changed these lines to use the names of the enumerators (FIRST, SECOND, ……). All the tests passed. Can you see why? Can you also see why each of these steps was necessary? The refactoring tool made sure that all previous callers of weekInMonthToString now called toString on the weekInMonth enumerator because all enumerators implement toString to simply return their names.…
接下来,我删掉了整个方法!有 5 个断言失败了(第 411 ~ 415 行,代码清单 B-4)。我改动了这些代码行,让它们使用枚举元素的名称(FIRST、SECOND……)。全部测试都通过了。你知道为什么吗?你能否知道为什么这些步骤都是必要的吗?重构工具确保之前对 weekInMonthToString 方法的调用现在都调用 weekInMonth 枚举元素的 toString 方法,全部枚举元素都以返回其名称的形式实现了 toString 方法……
Unfortunately, I was a bit too clever. As elegant as that wonderful chain of refactorings was, I finally realized that the only users of this function were the tests I had just modified, so I deleted the tests.
我不幸有点聪明过头了。这一套美妙的重构下来,我终于意识到,这个函数的唯一调用者,就是我刚修改的测试,所以我删除了这些测试。
Fool me once, shame on you. Fool me twice, shame on me! So after determining that nobody other than the tests called relativeToString (lines 765–781), I simply deleted the function and its tests.
愚我一次,是你之耻。愚我两次,是我之耻!所以,在判定除了测试之外没有人调用过 relativeToString(第 765 ~ 781 行)后,我就删除了该函数及其测试。
We have finally made it to the abstract methods of this abstract class. And the first one is as appropriate as they come: toSerial (lines 838–844). Back on page 279 I had changed the name to toOrdinal. Having looked at it in this context, I decided the name should be changed to getOrdinalDay.
我们最后将其改为这个抽象类的抽象方法。第一个函数保持了原样:toSerial(第 838 ~ 844 行)。前文我曾把名称改为 toOrdinal。以现在的情形看,我决定应该把名称改为 getOrdinalDay。
The next abstract method is toDate (lines 838–844). It converts a DayDate to a java.util.Date. Why is this method abstract? If we look at its implementation in SpreadsheetDate (lines 198–207, Listing B-5, page 382), we see that it doesn’t depend on anything in the implementation of that class [G6]. So I pushed it up.
下一个抽象方法是 toDate(第 838 ~ 844 行)。它将 DayDate 转换为 java.util.Date。这个方法为何是抽象的?查看其在 SpreadsheetDate 中的实现(第 198 ~ 207 行,代码清单 B-5),可以看到它并不依赖于该类的实现[G6]。所以,我把它往上推了。
The getYYYY, getMonth, and getDayOfMonth methods are nicely abstract. However, the getDayOfWeek method is another one that should be pulled up from SpreadSheetDate because it doesn’t depend on anything that can’t be found in DayDate [G6]. Or does it?
方法 getYYYY、getMonth 和 getDayOfMonth 已经是抽象方法。不过,getDayOfWeek 方法是另一个应该从 SpreadsheetDate 中提出来的方法,因为它不依赖于 DayDate 之外的东西[G6]。是这样吗?
If you look carefully (line 247, Listing B-5, page 382), you’ll see that the algorithm implicitly depends on the origin of the ordinal day (in other words, the day of the week of day 0). So even though this function has no physical dependencies that couldn’t be moved to DayDate, it does have a logical dependency.
仔细阅读(第 247 行,代码清单 B-5),可以发现该算法暗中依赖于顺序日期的起点(换言之,第 0 天的星期日数)。所以,即便该方法没有物理上的依赖,也不能移到 DayDate 中,因为它的确有逻辑上的依赖。
Logical dependencies like this bother me [G22]. If something logical depends on the implementation, then something physical should too. Also, it seems to me that the algorithm itself could be generic with a much smaller portion of it dependent on the implementation [G6].
这样的逻辑依赖困扰了我[G22]。如果有什么东西在逻辑上依赖实现的话,也该有什么物理上的依赖存在。我也认为,算法本身也该有一小部分依赖于实现。
So I created an abstract method in DayDate named getDayOfWeekForOrdinalZero and implemented it in SpreadsheetDate to return Day.SATURDAY. Then I moved the getDayOfWeek method up to DayDate and changed it to call getOrdinalDay and getDayOfWeekForOrdinal-Zero.
所以我在 DayDate 中创建了一个名为 getDayOfWekForOrdinalZero 的抽象方法,并在 SpreadsheetDate 中实现它,返回 Day.SATURDAY。然后我把 getDayOfWeek 上移到 DayDate 中,并调用 getOrdinalDay 和 getDayOfWeekForOrdinal Zero。
public Day getDayOfWeek() {
Day startingDay = getDayOfWeekForOrdinalZero();
int startingOffset = startingDay.index - Day.SUNDAY.index;
return Day.make((getOrdinalDay() + startingOffset) % 7 + 1);
}
2
3
4
5
As a side note, look carefully at the comment on line 895 through line 899. Was this repetition really necessary? As usual, I deleted this comment along with all the others.
顺便说一句,请仔细阅读第 895 ~ 899 行的注释。这样的重复有必要吗?通常,我会删除这类注释。
The next method is compare (lines 902–913). Again, this method is inappropriately abstract [G6], so I pulled the implementation up into DayDate. Also, the name does not communicate enough [N1]. This method actually returns the difference in days since the argument. So I changed the name to daysSince. Also, I noted that there weren’t any tests for this method, so I wrote them.
下一个方法是 compare(第 902 ~ 913 行)。同样,该抽象方法是不恰当的[G6]。我将其实现上移到 DayDate。其名称也不足够有沟通意义 [N1]。方法实际上返回的是自参数日期以来的天数,所以我把名称改为 daysSince。我还注意到该方法没有测试,就为它编写了测试。
The next six functions (lines 915–980) are all abstract methods that should be implemented in DayDate. So I pulled them all up from SpreadsheetDate.
下面 6 个函数(第 915 ~ 980 行)全都是应该在 DayDate 中实现的抽象方法。我把它们全都从 SpreadsheetDate 中抽出来了。
The last function, isInRange (lines 982–995) also needs to be pulled up and refactored. The switch statement is a bit ugly [G23] and can be replaced by moving the cases into the DateInterval enum.
最后一个函数 isInRange(第 982 ~ 995 行)也需要推到上一层并重构之。那个 switch 语句有点丑陋[G23],可以把那些条件判断移到 DateInterval 枚举中去。
public enum DateInterval {
OPEN {
public boolean isIn(int d, int left, int right) {
return d > left && d < right;
}
},
CLOSED_LEFT {
public boolean isIn(int d, int left, int right) {
return d >= left && d < right;
}
},
CLOSED_RIGHT {
public boolean isIn(int d, int left, int right) {
return d > left && d <= right;
}
},
CLOSED {
public boolean isIn(int d, int left, int right) {
return d >= left && d <= right;
}
};
public abstract boolean isIn(int d, int left, int right);
}
public boolean isInRange(DayDate d1, DayDate d2, DateInterval interval) {
int left = Math.min(d1.getOrdinalDay(), d2.getOrdinalDay());
int right = Math.max(d1.getOrdinalDay(), d2.getOrdinalDay());
return interval.isIn(getOrdinalDay(), left, right);
}
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
That brings us to the end of DayDate. So now we’ll make one more pass over the whole class to see how well it flows.
我们来到了 DayDate 的末尾。现在我们要从头到尾再过一次,看看> 整个重构过程是怎样良好执行的。
First, the opening comment is long out of date, so I shortened and improved it [C2].
首先,开端注释过时已久,我缩短并改进了它[C2]。
Next, I moved all the remaining enums out into their own files [G12].
然后,我把全部枚举移到它们自己的文件中[G12]。
Next, I moved the static variable (dateFormatSymbols) and three static methods (getMonthNames, isLeapYear, lastDayOfMonth) into a new class named DateUtil [G6].
跟着,我把静态变量(dateFormatSymbols)和 3 个静态方法(getMonthNames、isLeapYear 和 lastDayOfMonth)移到名为 DateUtil 的新类中[G6]。
I moved the abstract methods up to the top where they belong [G24].
我把那些抽象方法上移到它们该在的顶层类中[G24]。
I changed Month.make to Month.fromInt [N1] and did the same for all the other enums. I also created a toInt() accessor for all the enums and made the index field private.
我把 Month.make 改为 Month.fromInt [N1],并如法炮制所有其他枚举。我还为全部枚举创建了 toInt( )访问器,把 index 字段改为私有。
There was some interesting duplication [G5] in plusYears and plusMonths that I was able to eliminate by extracting a new method named correctLastDayOfMonth, making the all three methods much clearer.
在 plusYears 和 plusMonths 中存在一些有趣的重复[G5],我通过抽离出名为 correctLastDayOfMonth 的新方法消解了重复,使这 3 个方法清晰多了。
I got rid of the magic number 1 [G25], replacing it with Month.JANUARY.toInt() or Day.SUNDAY.toInt(), as appropriate. I spent a little time with SpreadsheetDate, cleaning up the algorithms a bit. The end result is contained in Listing B-7, page 394, through Listing B-16, page 405.
我消除了魔术数 1 [G25],用 Month.JANUARY.toInt( )或 Day.SUNDAY.toInt( )做了恰当的替换。我在 SpreadsheetDate 上花了点时间,清理了一下算法。最终结果在代码清单 B-7 ~ 16 中。
Interestingly the code coverage in DayDate has decreased to 84.9 percent! This is not because less functionality is being tested; rather it is because the class has shrunk so much that the few uncovered lines have a greater weight. DayDate now has 45 out of 53 executable statements covered by tests. The uncovered lines are so trivial that they weren’t worth testing.
有趣的是,DayDate 的代码覆盖率降低到了 84.9%!这并不是因为测试到的功能减少了,而是因为该类缩减得太多,导致少量未覆盖到的代码行拥有了更大权重。DayDate 的 53 个可执行语句中有 45 个得到测试覆盖。未覆盖的代码行微细到不值得测试。
# 16.3 CONCLUSION 小结
So once again we’ve followed the Boy Scout Rule. We’ve checked the code in a bit cleaner than when we checked it out. It took a little time, but it was worth it. Test coverage was increased, some bugs were fixed, the code was clarified and shrunk. The next person to look at this code will hopefully find it easier to deal with than we did. That person will also probably be able to clean it up a bit more than we did.
我们再一次遵从了童子军军规。我们签入的代码,要比签出时整洁了一点。虽然花了点时间,不过很值得。测试覆盖率提升了,修改了一些缺陷,代码清晰并缩短了。后来者有望比我们更容易地应付这些代码。他也有可能把代码整理得更干净些。