`

superword中一次精彩的重构

阅读更多

我们先来看看需要重构的功能是一个下拉选择框,可任意选择11部词典中的一部,访问地址:http://123.56.99.179/select/dictionary-select.jsp?dict=RANDOMHOUSE,在HTML中的效果如下图所示:

HTML代码如下:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
<select name="dict" id="dict" onchange="update();">
 
    <option value="ICIBA">iCIBA</option>
    <option value="YOUDAO">Youdao</option>
    <option value="COLLINS">Collins</option>
    <option value="WEBSTER">Webster's</option>
    <option value="OXFORD">Oxford</option>
    <option value="CAMBRIDGE">Cambridge</option>
    <option value="MACMILLAN">Macmillan</option>
    <option value="HERITAGE">Heritage</option>
    <option value="WIKTIONARY">Wiktionary</option>
    <option value="WORDNET">WordNet</option>
    <option value="RANDOMHOUSE" selected="selected">RandomHouse</option>
 
</select>

 

我们接下来看看最初的JSP代码是如何实现这个功能的:

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
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
<%@ page contentType="text/html;charset=UTF-8" language="java" %>
<%@ page import="org.apdplat.superword.tools.WordLinker" %>
<%@ page import="org.apdplat.superword.tools.WordLinker.Dictionary" %>
 
        <select name="dict" id="dict" onchange="update();">
    <%
        if (Dictionary.ICIBA==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA" selected="selected">爱词霸</option>
            <option value="YOUDAO">有道</option>
            <option value="COLLINS">柯林斯</option>
            <option value="WEBSTER">韦氏</option>
            <option value="OXFORD">牛津</option>
            <option value="CAMBRIDGE">剑桥</option>
            <option value="MACMILLAN">麦克米伦</option>
            <option value="HERITAGE">美国传统</option>
            <option value="WIKTIONARY">维基词典</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.YOUDAO==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">爱词霸</option>
            <option value="YOUDAO" selected="selected">有道</option>
            <option value="COLLINS">柯林斯</option>
            <option value="WEBSTER">韦氏</option>
            <option value="OXFORD">牛津</option>
            <option value="CAMBRIDGE">剑桥</option>
            <option value="MACMILLAN">麦克米伦</option>
            <option value="HERITAGE">美国传统</option>
            <option value="WIKTIONARY">维基词典</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.COLLINS==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">爱词霸</option>
            <option value="YOUDAO">有道</option>
            <option value="COLLINS" selected="selected">柯林斯</option>
            <option value="WEBSTER">韦氏</option>
            <option value="OXFORD">牛津</option>
            <option value="CAMBRIDGE">剑桥</option>
            <option value="MACMILLAN">麦克米伦</option>
            <option value="HERITAGE">美国传统</option>
            <option value="WIKTIONARY">维基词典</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.WEBSTER==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">爱词霸</option>
            <option value="YOUDAO">有道</option>
            <option value="COLLINS">柯林斯</option>
            <option value="WEBSTER" selected="selected">韦氏</option>
            <option value="OXFORD">牛津</option>
            <option value="CAMBRIDGE">剑桥</option>
            <option value="MACMILLAN">麦克米伦</option>
            <option value="HERITAGE">美国传统</option>
            <option value="WIKTIONARY">维基词典</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.OXFORD==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">爱词霸</option>
            <option value="YOUDAO">有道</option>
            <option value="COLLINS">柯林斯</option>
            <option value="WEBSTER">韦氏</option>
            <option value="OXFORD" selected="selected">牛津</option>
            <option value="CAMBRIDGE">剑桥</option>
            <option value="MACMILLAN">麦克米伦</option>
            <option value="HERITAGE">美国传统</option>
            <option value="WIKTIONARY">维基词典</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.CAMBRIDGE==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">爱词霸</option>
            <option value="YOUDAO">有道</option>
            <option value="COLLINS">柯林斯</option>
            <option value="WEBSTER">韦氏</option>
            <option value="OXFORD">牛津</option>
            <option value="CAMBRIDGE" selected="selected">剑桥</option>
            <option value="MACMILLAN">麦克米伦</option>
            <option value="HERITAGE">美国传统</option>
            <option value="WIKTIONARY">维基词典</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.MACMILLAN==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">爱词霸</option>
            <option value="YOUDAO">有道</option>
            <option value="COLLINS">柯林斯</option>
            <option value="WEBSTER">韦氏</option>
            <option value="OXFORD">牛津</option>
            <option value="CAMBRIDGE">剑桥</option>
            <option value="MACMILLAN" selected="selected">麦克米伦</option>
            <option value="HERITAGE">美国传统</option>
            <option value="WIKTIONARY">维基词典</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.HERITAGE==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">爱词霸</option>
            <option value="YOUDAO">有道</option>
            <option value="COLLINS">柯林斯</option>
            <option value="WEBSTER">韦氏</option>
            <option value="OXFORD">牛津</option>
            <option value="CAMBRIDGE">剑桥</option>
            <option value="MACMILLAN">麦克米伦</option>
            <option value="HERITAGE" selected="selected">美国传统</option>
            <option value="WIKTIONARY">维基词典</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.WIKTIONARY==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">爱词霸</option>
            <option value="YOUDAO">有道</option>
            <option value="COLLINS">柯林斯</option>
            <option value="WEBSTER">韦氏</option>
            <option value="OXFORD">牛津</option>
            <option value="CAMBRIDGE">剑桥</option>
            <option value="MACMILLAN">麦克米伦</option>
            <option value="HERITAGE">美国传统</option>
            <option value="WIKTIONARY" selected="selected">维基词典</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
            <%
    else if (Dictionary.WORDNET==WordLinker.getValidDictionary(request.getParameter("dict"))) {
            %>
            <option value="ICIBA">爱词霸</option>
            <option value="YOUDAO">有道</option>
            <option value="COLLINS">柯林斯</option>
            <option value="WEBSTER">韦氏</option>
            <option value="OXFORD">牛津</option>
            <option value="CAMBRIDGE">剑桥</option>
            <option value="MACMILLAN">麦克米伦</option>
            <option value="HERITAGE">美国传统</option>
            <option value="WIKTIONARY">维基词典</option>
            <option value="WORDNET" selected="selected">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.RANDOMHOUSE==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">爱词霸</option>
            <option value="YOUDAO">有道</option>
            <option value="COLLINS">柯林斯</option>
            <option value="WEBSTER">韦氏</option>
            <option value="OXFORD">牛津</option>
            <option value="CAMBRIDGE">剑桥</option>
            <option value="MACMILLAN">麦克米伦</option>
            <option value="HERITAGE">美国传统</option>
            <option value="WIKTIONARY">维基词典</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE" selected="selected">RandomHouse</option>
    <%
    }
    %>
        </select>

 

 

这段代码有什么问题呢?

如果我们一次写好后就不需要维护和更改这段代码,那么根据自己的经验知识和理解快速实现功能,满足要求即可。

但是,如果需要维护代码和扩展功能呢?比如,我现在要把选项显示的所有的中文全部改为英文,那么就变成如下代码了:

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
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
        <select name="dict" id="dict" onchange="update();">
    <%
        if (Dictionary.ICIBA==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA" selected="selected">iCIBA</option>
            <option value="YOUDAO">Youdao</option>
            <option value="COLLINS">Collins</option>
            <option value="WEBSTER">Webster's</option>
            <option value="OXFORD">Oxford</option>
            <option value="CAMBRIDGE">Cambridge</option>
            <option value="MACMILLAN">Macmillan</option>
            <option value="HERITAGE">Heritage</option>
            <option value="WIKTIONARY">Wiktionary</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.YOUDAO==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">iCIBA</option>
            <option value="YOUDAO" selected="selected">Youdao</option>
            <option value="COLLINS">Collins</option>
            <option value="WEBSTER">Webster's</option>
            <option value="OXFORD">Oxford</option>
            <option value="CAMBRIDGE">Cambridge</option>
            <option value="MACMILLAN">Macmillan</option>
            <option value="HERITAGE">Heritage</option>
            <option value="WIKTIONARY">Wiktionary</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.COLLINS==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">iCIBA</option>
            <option value="YOUDAO">Youdao</option>
            <option value="COLLINS" selected="selected">Collins</option>
            <option value="WEBSTER">Webster's</option>
            <option value="OXFORD">Oxford</option>
            <option value="CAMBRIDGE">Cambridge</option>
            <option value="MACMILLAN">Macmillan</option>
            <option value="HERITAGE">Heritage</option>
            <option value="WIKTIONARY">Wiktionary</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.WEBSTER==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">iCIBA</option>
            <option value="YOUDAO">Youdao</option>
            <option value="COLLINS">Collins</option>
            <option value="WEBSTER" selected="selected">Webster's</option>
            <option value="OXFORD">Oxford</option>
            <option value="CAMBRIDGE">Cambridge</option>
            <option value="MACMILLAN">Macmillan</option>
            <option value="HERITAGE">Heritage</option>
            <option value="WIKTIONARY">Wiktionary</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.OXFORD==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">iCIBA</option>
            <option value="YOUDAO">Youdao</option>
            <option value="COLLINS">Collins</option>
            <option value="WEBSTER">Webster's</option>
            <option value="OXFORD" selected="selected">Oxford</option>
            <option value="CAMBRIDGE">Cambridge</option>
            <option value="MACMILLAN">Macmillan</option>
            <option value="HERITAGE">Heritage</option>
            <option value="WIKTIONARY">Wiktionary</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.CAMBRIDGE==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">iCIBA</option>
            <option value="YOUDAO">Youdao</option>
            <option value="COLLINS">Collins</option>
            <option value="WEBSTER">Webster's</option>
            <option value="OXFORD">Oxford</option>
            <option value="CAMBRIDGE" selected="selected">Cambridge</option>
            <option value="MACMILLAN">Macmillan</option>
            <option value="HERITAGE">Heritage</option>
            <option value="WIKTIONARY">Wiktionary</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.MACMILLAN==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">iCIBA</option>
            <option value="YOUDAO">Youdao</option>
            <option value="COLLINS">Collins</option>
            <option value="WEBSTER">Webster's</option>
            <option value="OXFORD">Oxford</option>
            <option value="CAMBRIDGE">Cambridge</option>
            <option value="MACMILLAN" selected="selected">Macmillan</option>
            <option value="HERITAGE">Heritage</option>
            <option value="WIKTIONARY">Wiktionary</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.HERITAGE==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">iCIBA</option>
            <option value="YOUDAO">Youdao</option>
            <option value="COLLINS">Collins</option>
            <option value="WEBSTER">Webster's</option>
            <option value="OXFORD">Oxford</option>
            <option value="CAMBRIDGE">Cambridge</option>
            <option value="MACMILLAN">Macmillan</option>
            <option value="HERITAGE" selected="selected">Heritage</option>
            <option value="WIKTIONARY">Wiktionary</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.WIKTIONARY==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">iCIBA</option>
            <option value="YOUDAO">Youdao</option>
            <option value="COLLINS">Collins</option>
            <option value="WEBSTER">Webster's</option>
            <option value="OXFORD">Oxford</option>
            <option value="CAMBRIDGE">Cambridge</option>
            <option value="MACMILLAN">Macmillan</option>
            <option value="HERITAGE">Heritage</option>
            <option value="WIKTIONARY" selected="selected">Wiktionary</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
            <%
    else if (Dictionary.WORDNET==WordLinker.getValidDictionary(request.getParameter("dict"))) {
            %>
            <option value="ICIBA">iCIBA</option>
            <option value="YOUDAO">Youdao</option>
            <option value="COLLINS">Collins</option>
            <option value="WEBSTER">Webster's</option>
            <option value="OXFORD">Oxford</option>
            <option value="CAMBRIDGE">Cambridge</option>
            <option value="MACMILLAN">Macmillan</option>
            <option value="HERITAGE">Heritage</option>
            <option value="WIKTIONARY">Wiktionary</option>
            <option value="WORDNET" selected="selected">WordNet</option>
            <option value="RANDOMHOUSE">RandomHouse</option>
    <%
    else if (Dictionary.RANDOMHOUSE==WordLinker.getValidDictionary(request.getParameter("dict"))) {
    %>
            <option value="ICIBA">iCIBA</option>
            <option value="YOUDAO">Youdao</option>
            <option value="COLLINS">Collins</option>
            <option value="WEBSTER">Webster's</option>
            <option value="OXFORD">Oxford</option>
            <option value="CAMBRIDGE">Cambridge</option>
            <option value="MACMILLAN">Macmillan</option>
            <option value="HERITAGE">Heritage</option>
            <option value="WIKTIONARY">Wiktionary</option>
            <option value="WORDNET">WordNet</option>
            <option value="RANDOMHOUSE" selected="selected">RandomHouse</option>
    <%
    }
    %>
        </select>

 

 

你可能说,我只需要执行 CTRL+R 11次,没啥问题啊?其实这恰恰就是问题,因为同样的字符串重复了11次。

改成了英文名称之后,接下来,如果我们要新增一个选项怎么办呢?我们不但需要复制粘贴代码来新增一个ELSE IF代码块,我们还需要修改既有的11个IF代码块,都新增一个option。这不但违反了DRY原则,也违反了开闭原则。下面开始来改造,改造后的代码如下:

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
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
    <select name="dict" id="dict" onchange="update();">
        <%
        if (Dictionary.ICIBA==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="ICIBA" selected="selected">iCIBA</option>
        <%
        else {
        %>
        <option value="ICIBA">iCIBA</option>
        <%
        }
        if (Dictionary.YOUDAO==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="YOUDAO" selected="selected">Youdao</option>
        <%
        else {
        %>
        <option value="YOUDAO">Youdao</option>
        <%
        }
        if (Dictionary.COLLINS==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="COLLINS" selected="selected">Collins</option>
        <%
        else {
        %>
        <option value="COLLINS">Collins</option>
        <%
        }
        if (Dictionary.WEBSTER==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="WEBSTER" selected="selected">Webster's</option>
        <%
        else {
        %>
        <option value="WEBSTER">Webster's</option>
        <%
        }
        if (Dictionary.OXFORD==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="OXFORD" selected="selected">Oxford</option>
        <%
        else {
        %>
        <option value="OXFORD">Oxford</option>
        <%
        }
        if (Dictionary.CAMBRIDGE==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="CAMBRIDGE" selected="selected">Cambridge</option>
        <%
        else {
        %>
        <option value="CAMBRIDGE">Cambridge</option>
        <%
        }
        if (Dictionary.MACMILLAN==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="MACMILLAN" selected="selected">Macmillan</option>
        <%
        else {
        %>
        <option value="MACMILLAN">Macmillan</option>
        <%
        }
        if (Dictionary.HERITAGE==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="HERITAGE" selected="selected">Heritage</option>
        <%
        else {
        %>
        <option value="HERITAGE">Heritage</option>
        <%
        }
        if (Dictionary.WIKTIONARY==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="WIKTIONARY" selected="selected">Wiktionary</option>
        <%
        else {
        %>
        <option value="WIKTIONARY">Wiktionary</option>
        <%
        }
        if (Dictionary.WORDNET==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="WORDNET" selected="selected">WordNet</option>
        <%
        else {
        %>
        <option value="WORDNET">WordNet</option>
        <%
        }
        if (Dictionary.RANDOMHOUSE==WordLinker.getValidDictionary(request.getParameter("dict"))) {
        %>
        <option value="RANDOMHOUSE" selected="selected">RandomHouse</option>
        <%
        else {
        %>
        <option value="RANDOMHOUSE">RandomHouse</option>
        <%
        }
        %>
    </select>

 

 

重构代码后是不是可读性降低了?原来的代码虽然违反了DRY原则开闭原则,但是可读性却是极好的。即使如此,重构后的代码却是非常容易扩展,新增选项只需要增加一个IF代码块即可,不需要修改现有的IF代码块,符合了开闭原则,同时移除了重复了11次的option,所以虽然可读性降低了,但是却完美达成了DRY原则开闭原则

这个例子说明了一个非常重要的道理,具体的东西更容易让人理解,而更抽象的东西理解起来就要更费劲,所以,我们在写代码的时候,一开始完全没有必要满脑子的各种最佳实践,各种设计模式,各种架构模式,最佳实践设计模式架构模式都有限制的场景,他们都应该是重构自然而然的结果,一开始就考虑这些东西,不但会拖慢功能实现的速度,而且往往会陷于“过度设计”“不成熟的优化”的泥淖。

上面的代码还不够彻底,还存在重复,接着重构来贯彻DRY原则

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
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
    <select name="dict" id="dict" onchange="update();">
        <%
        Dictionary selectedDictionary = WordLinker.getValidDictionary(request.getParameter("dict"));
        if (Dictionary.ICIBA == selectedDictionary) {
        %>
            <option value="ICIBA" selected="selected">iCIBA</option>
        <%
        else {
        %>
            <option value="ICIBA">iCIBA</option>
        <%
        }
 
        if (Dictionary.YOUDAO == selectedDictionary) {
        %>
            <option value="YOUDAO" selected="selected">Youdao</option>
        <%
        else {
        %>
            <option value="YOUDAO">Youdao</option>
        <%
        }
 
        if (Dictionary.COLLINS == selectedDictionary) {
        %>
            <option value="COLLINS" selected="selected">Collins</option>
        <%
        else {
        %>
            <option value="COLLINS">Collins</option>
        <%
        }
 
        if (Dictionary.WEBSTER == selectedDictionary) {
        %>
            <option value="WEBSTER" selected="selected">Webster's</option>
        <%
        else {
        %>
            <option value="WEBSTER">Webster's</option>
        <%
        }
 
        if (Dictionary.OXFORD == selectedDictionary) {
        %>
            <option value="OXFORD" selected="selected">Oxford</option>
        <%
        else {
        %>
            <option value="OXFORD">Oxford</option>
        <%
        }
 
        if (Dictionary.CAMBRIDGE == selectedDictionary) {
        %>
            <option value="CAMBRIDGE" selected="selected">Cambridge</option>
        <%
        else {
        %>
            <option value="CAMBRIDGE">Cambridge</option>
        <%
        }
 
        if (Dictionary.MACMILLAN == selectedDictionary) {
        %>
            <option value="MACMILLAN" selected="selected">Macmillan</option>
        <%
        else {
        %>
            <option value="MACMILLAN">Macmillan</option>
        <%
        }
 
        if (Dictionary.HERITAGE == selectedDictionary) {
        %>
            <option value="HERITAGE" selected="selected">Heritage</option>
        <%
        else {
        %>
            <option value="HERITAGE">Heritage</option>
        <%
        }
 
        if (Dictionary.WIKTIONARY == selectedDictionary) {
        %>
            <option value="WIKTIONARY" selected="selected">Wiktionary</option>
        <%
        else {
        %>
            <option value="WIKTIONARY">Wiktionary</option>
        <%
        }
 
        if (Dictionary.WORDNET == selectedDictionary) {
        %>
            <option value="WORDNET" selected="selected">WordNet</option>
        <%
        else {
        %>
            <option value="WORDNET">WordNet</option>
        <%
        }
 
        if (Dictionary.RANDOMHOUSE == selectedDictionary) {
        %>
            <option value="RANDOMHOUSE" selected="selected">RandomHouse</option>
        <%
        else {
        %>
            <option value="RANDOMHOUSE">RandomHouse</option>
        <%
        }
        %>
    </select>

 

 

这次,我们把重复的代码抽取为一个变量,并在IF代码块前面加了一个空行,看上去是不是可读性提高了?这也说明了DRY原则对于代码的简洁易读是有帮助的,同时也说明了我们编写的代码的换行缩进等风格对可读性也是有影响的,作为一个有尊严的程序员,需要注意这些细节,细节决定成败,成功者往往只是比对手多做了一点而已,而多做的这一点,往往就是在细节上面。

虽然易读性有了提高,但是还是比较糟糕,离生成的HTML的差距还很大,下面继续重构,结果如下:

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
<%
    Dictionary selectedDictionary = WordLinker.getValidDictionary(request.getParameter("dict"));
    String ICIBA = "";
    String YOUDAO = "";
    String COLLINS = "";
    String WEBSTER = "";
    String OXFORD = "";
    String CAMBRIDGE = "";
    String MACMILLAN = "";
    String HERITAGE = "";
    String WIKTIONARY = "";
    String WORDNET = "";
    String RANDOMHOUSE = "";
    String selected = "selected=\"selected\"";
    switch (selectedDictionary){
        case ICIBA:
            ICIBA = selected; break;
        case YOUDAO:
            YOUDAO = selected; break;
        case COLLINS:
            COLLINS = selected; break;
        case WEBSTER:
            WEBSTER = selected; break;
        case OXFORD:
            OXFORD = selected; break;
        case CAMBRIDGE:
            CAMBRIDGE = selected; break;
        case MACMILLAN:
            MACMILLAN = selected; break;
        case HERITAGE:
            HERITAGE = selected; break;
        case WIKTIONARY:
            WIKTIONARY = selected; break;
        case WORDNET:
            WORDNET = selected; break;
        case RANDOMHOUSE:
            RANDOMHOUSE = selected; break;
    }
%>
<select name="dict" id="dict" onchange="update();">
    <option value="ICIBA" <%=ICIBA%>>iCIBA</option>
    <option value="YOUDAO" <%=YOUDAO%>>Youdao</option>
    <option value="COLLINS" <%=COLLINS%>>Collins</option>
    <option value="WEBSTER" <%=WEBSTER%>>Webster's</option>
    <option value="OXFORD" <%=OXFORD%>>Oxford</option>
    <option value="CAMBRIDGE" <%=CAMBRIDGE%>>Cambridge</option>
    <option value="MACMILLAN" <%=MACMILLAN%>>Macmillan</option>
    <option value="HERITAGE" <%=HERITAGE%>>Heritage</option>
    <option value="WIKTIONARY" <%=WIKTIONARY%>>Wiktionary</option>
    <option value="WORDNET" <%=WORDNET%>>WordNet</option>
    <option value="RANDOMHOUSE" <%=RANDOMHOUSE%>>RandomHouse</option>
</select>

 

 

这次重构过后,看起来是不是跟最终生成的HTML差别已经不大了?易读性有了质的飞跃,但是你有没有发现,这里面还是有很多重复,而且存在着模式和规律,我们可以再继续重构吗?

肯定可以,直接看重构后的结果吧:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
    <select name="dict" id="dict" onchange="update();">
        <%
        Dictionary selectedDictionary = WordLinker.getValidDictionary(request.getParameter("dict"));
        for(Dictionary dictionary : Dictionary.values()){
            if (dictionary == selectedDictionary) {
        %>
            <option value="<%=dictionary.name()%>" selected="selected"><%=dictionary.getDes()%></option>
        <%
            else {
        %>
            <option value="<%=dictionary.name()%>"><%=dictionary.getDes()%></option>
        <%
            }
        }
        %>
    </select>

 

 

这次,我们直接遍历数据模型Dictionary,从而完全消除了重复,同时在开闭原则上也有了新的高度,以后当我们需要扩展新的选项的时候,我们已经不再需要修改哪怕一行代码了。

 

那如果我们不是需要Dictionary的所有选项怎么办呢?这肯定也不是问题,解决方法如下:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
<%!
    private static final List<Dictionary> DICTIONARIES = Arrays.asList(Dictionary.ICIBA, Dictionary.YOUDAO, Dictionary.WEBSTER, Dictionary.OXFORD);
%>
<select name="dictionary" id="dictionary" onchange="update();">
    <%
        Dictionary selectedDictionary = WordLinker.getValidDictionary(request.getParameter("dictionary"));
        for(Dictionary dictionary : DICTIONARIES){
            if (dictionary == selectedDictionary) {
    %>
    <option value="<%=dictionary.name()%>" selected="selected"><%=dictionary.getDes()%></option>
    <%
    else {
    %>
    <option value="<%=dictionary.name()%>"><%=dictionary.getDes()%></option>
    <%
            }
        }
    %>
</select>

 

 

一次完美的重构就此结束,要想尝试更多的重构工作,敬请加入到superword的重构中来:https://github.com/ysc/superword

 

 

 

 

 

 

 

 

1
2
分享到:
评论

相关推荐

Global site tag (gtag.js) - Google Analytics