目录
前言
并发编程
JUnit框架
重构策略
1. 注释
2.函数
3.一般性问题
3.Java和名称
5.测试
前言
本文是《代码整洁之道》读书笔记的下篇,聚焦于并发编程、实战之JUnit框架重构和重构策略。
上篇地址为: >>>
并发编程
1.并发帮助我们把做什么(目的)和何时做(时机)分解开。多线程可能会导致数据不一致问题。参考:并发编程入门(二):Sychronized与线程间通信
2.并发防御原则:
单一权责原则:方法/类/组件应当只有一个修改的理由。建议:分离并发相关代码和其他代码。
限制数据作用域:两个线程修改共享对象同一字段,可能会互相干扰。解决方案为Sychronized保护共享对象临界区。建议:谨记数据封装,尽可能减少同步区域、严格限制对可能被共享数据的访问。
线程应尽可能独立:每个线程尽量不与其他线程共享数据。
3.Java类库:有一些线程安全群集。比如ConcurrentHashMap、ReentrantLock、Semaphore和CountDownLatch等。
4.HashMap、HashTable和ConcurrentHashMap:区别。
//重构前:单个方法线程安全,组合线程不安全
if(!hashTable.containsKey(someKey)) {
hashTable.put(someKey, new SomeValue());
}
//重构后:
//1.锁定HashTable,确定其他使用者做了基于客户端的锁定
synchronized(map) {
if(!map.containsKey(key))
map.put(key,value);
}
//2.Adapter适配:对象封装Hashtable
public class WrappedHashtable<K, V> {
private Map<K, V> map = new Hashtable<K, V>();
public synchronized void putIfAbsent(K key, V value) {
if (map.containsKey(key))
map.put(key, value);
}
}
//3.采用线程安全的合集
ConcurrentHashMap<Integer, String> map = new ConcurrentHashMap<Integer, String>();
map.putIfAbsent(key, value);
JUnit框架
以JUnit代码为例进行了优化:
原始版本:
public class ComparisonCompactor {
private static final String ELLIPSIS = "...";
private static final String DELTA_END = "]";
private static final String DELTA_START = "[";
private int fContextLength;
private String fExpected;
private String fActual;
private int fPrefix;
private int fSuffix;
public ComparisonCompactor(int contextLength,
String expected,
String actual) {
fContextLength = contextLength;
fExpected = expected;
fActual = actual;
}
public String compact(String message) {
if (fExpected == null || fActual == null || areStringsEqual())
return Assert.format(message, fExpected, fActual);
findCommonPrefix();
findCommonSuffix();
String expected = compactString(fExpected);
String actual = compactString(fActual);
return Assert.format(message, expected, actual);
}
private String compactString(String source) {
String result = DELTA_START +
source.substring(fPrefix, source.length() -
fSuffix + 1) + DELTA_END;
if (fPrefix > 0)
result = computeCommonPrefix() + result;
if (fSuffix > 0)
result = result + computeCommonSuffix();
return result;
}
private void findCommonPrefix() {
fPrefix = 0;
int end = Math.min(fExpected.length(), fActual.length());
for (; fPrefix < end; fPrefix++) {
if (fExpected.charAt(fPrefix) != fActual.charAt(fPrefix))
break;
}
}
private void findCommonSuffix() {
int expectedSuffix = fExpected.length() - 1;
int actualSuffix = fActual.length() - 1;
for (;
actualSuffix >= fPrefix && expectedSuffix >= fPrefix;
actualSuffix--, expectedSuffix--) {
if (fExpected.charAt(expectedSuffix) != fActual.charAt(actualSuffix))
break;
}
fSuffix = fExpected.length() - expectedSuffix;
}
private String computeCommonPrefix() {
return (fPrefix > fContextLength ? ELLIPSIS : "") +
fExpected.substring(Math.max(0, fPrefix - fContextLength),
fPrefix);
}
private String computeCommonSuffix() {
int end = Math.min(fExpected.length() - fSuffix + 1 + fContextLength,
fExpected.length());
return fExpected.substring(fExpected.length() - fSuffix + 1, end) +
(fExpected.length() - fSuffix + 1 < fExpected.length() -
fContextLength ? ELLIPSIS : "");
}
private boolean areStringsEqual() {
return fExpected.equals(fActual);
}
}
重构后版本:
package junit.framework;
public class ComparisonCompactor {
private static final String ELLIPSIS = "...";
private static final String DELTA_END = "]";
private static final String DELTA_START = "[";
private int contextLength;
private String expected;
private String actual;
private int prefixLength;
private int suffixLength;
public ComparisonCompactor(
int contextLength, String expected, String actual
) {
this.contextLength = contextLength;
this.expected = expected;
this.actual = actual;
}
public String formatCompactedComparison(String message) {
String compactExpected = expected;
String compactActual = actual;
if (shouldBeCompacted()) {
findCommonPrefixAndSuffix();
compactExpected = compact(expected);
compactActual = compact(actual);
}
return Assert.format(message, compactExpected, compactActual);
}
private boolean shouldBeCompacted() {
return !shouldNotBeCompacted();
}
private boolean shouldNotBeCompacted() {
return expected == null ||
actual == null ||
expected.equals(actual);
}
private void findCommonPrefixAndSuffix() {
findCommonPrefix();
suffixLength = 0;
for (; !suffixOverlapsPrefix(); suffixLength++) {
if (charFromEnd(expected, suffixLength) !=
charFromEnd(actual, suffixLength)
)
break;
}
}
private char charFromEnd(String s, int i) {
return s.charAt(s.length() - i - 1);
}
private boolean suffixOverlapsPrefix() {
return actual.length() - suffixLength <= prefixLength ||
expected.length() - suffixLength <= prefixLength;
}
private void findCommonPrefix() {
prefixLength = 0;
int end = Math.min(expected.length(), actual.length());
for (; prefixLength < end; prefixLength++)
if (expected.charAt(prefixLength) != actual.charAt (prefixLength))
break;
}
private String compact(String s) {
return new StringBuilder()
.append(startingEllipsis())
.append(startingContext())
.append(DELTA_START)
.append(delta(s))
.append(DELTA_END)
.append(endingContext())
.append(endingEllipsis())
.toString();
}
private String startingEllipsis() {
return prefixLength > contextLength ? ELLIPSIS : "";
}
private String startingContext() {
int contextStart = Math.max(0, prefixLength - contextLength);
int contextEnd = prefixLength;
return expected.substring(contextStart, contextEnd);
}
private String delta(String s) {
int deltaStart = prefixLength;
int deltaEnd = s.length() - suffixLength;
return s.substring(deltaStart, deltaEnd);
}
private String endingContext() {
int contextStart = expected.length() - suffixLength;
int contextEnd =
Math.min(contextStart + contextLength, expected.length());
return expected.substring(contextStart, contextEnd);
}
private String endingEllipsis() {
return (suffixLength > contextLength ? ELLIPSIS : "");
}
}
重构点有:
封装条件判断
//优化前
if (expected == null || actual == null || areStringsEqual())
return Assert.format(message, expected, actual);
//优化后:抽离方法,否定式比肯定式难理解一些。
if (canBeCompacted())
return Assert.format(message, expected, actual);
private boolean canBeCompacted() {
return expected != null && actual != null && !areStringsEqual();
}
函数单一职责
//优化前
public String compact(String message) {
if (canBeCompacted()) {
findCommonPrefix();
findCommonSuffix();
String compactExpected = compactString(expected);
String compactActual = compactString(actual);
return Assert.format(message, compactExpected, compactActual);
} else {
return Assert.format(message, expected, actual);
}
}
private boolean canBeCompacted() {
return expected != null && actual != null && !areStringsEqual();
}
//优化后:compactXX函数除了压缩,什么也不做。
...
private String compactExpected;
private String compactActual;
...
public String formatCompactedComparison(String message) {
if (canBeCompacted()) {
compactExpectedAndActual();
return Assert.format(message, compactExpected, compactActual);
} else {
return Assert.format(message, expected, actual);
}
}
private void compactExpectedAndActual() {
findCommonPrefix();
findCommonSuffix();
compactExpected = compactString(expected);
compactActual = compactString(actual);
}
时序性耦合问题
//优化前
private void compactExpectedAndActual() {
prefixIndex = findCommonPrefix();
suffixIndex = findCommonSuffix(prefixIndex);
compactExpected = compactString(expected);
compactActual = compactString(actual);
}
private int findCommonSuffix(int prefixIndex) {
int expectedSuffix = expected.length() - 1;
int actualSuffix = actual.length() - 1;
for (; actualSuffix >= prefixIndex && expectedSuffix >= prefixIndex;
actualSuffix--, expectedSuffix--) {
if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix))
break;
}
return expected.length() - expectedSuffix;
}
//优化后:findCommonSuffix依赖prefixIndex,但后者是由findCommonSuffix计算而来。
private void compactExpectedAndActual() {
findCommonPrefixAndSuffix();
compactExpected = compactString(expected);
compactActual = compactString(actual);
}
private void findCommonPrefixAndSuffix() {
findCommonPrefix();
int expectedSuffix = expected.length() - 1;
int actualSuffix = actual.length() - 1;
for (;
actualSuffix >= prefixIndex && expectedSuffix >= prefixIndex;
actualSuffix--, expectedSuffix--
) {
if (expected.charAt(expectedSuffix) != actual.charAt(actualSuffix))
break;
}
suffixIndex = expected.length() - expectedSuffix;
}
private void findCommonPrefix() {
prefixIndex = 0;
int end = Math.min(expected.length(), actual.length());
for (; prefixIndex < end; prefixIndex++)
if (expected.charAt(prefixIndex) != actual.charAt(prefixIndex))
break;
}
代码逻辑优化
//优化后:用charFormEnd的-1替代computeCommonSuffix的一堆+1。
private String computeCommonSuffix() {
int end = Math.min(expected.length() - suffixLength +
contextLength, expected.length()
);
return
expected.substring(expected.length() - suffixLength, end) +
(expected.length() - suffixLength <
expected.length() - contextLength ?
ELLIPSIS : "");
}
if语句优化
//优化后:删除无用if语句
private String compactString(String source) {
return
computeCommonPrefix() +
DELTA_START +
source.substring(prefixLength, source.length() - suffixLength) +
DELTA_END +
computeCommonSuffix();
}
重构策略
1. 注释
不恰当信息:修改时间等易过时信息不应在注释出现,只应该描述代码和设计技术性信息。
废弃的注释:别编写将被废弃的注释,如发现,删除或更新。
冗余注释:注释应当谈及代码未提到的东西。
糟糕的注释:注释应字斟句酌,保持简洁。
注释掉的代码:注释掉的代码,应当删除。
2.函数
过多的参数:尽可能少,没参数最好,避免三个以上的参数。
输出参数:违反直觉,函数非要修改东西状态,就修改他所在对象的状态。
//优化前
appendFooters(s);
//优化后
report.appendFooter();
标识参数:布尔值参数告诉函数不止做了一件事,消灭。
//优化前:
render(Boolean isSuite)
//优化后
renderForSuite();
renderForSingleTest();
死函数:永不调用就删掉。
3.一般性问题
一个源文件中存在多种语言:理想源文件包括且只包括一种语言,比如Java源文件不应该把偶偶HTML、JS等;
明显的行为未被实现:函数或类应当符合人类直觉,实现别人期待的行为。
//期望字符串Mondy被翻译为Day.MONDAY,期望常用缩写能被翻译;期望函数忽略大小写。
Day day = DayDate.StringToDay(String dayName)
不正确的边界行为:谨小慎微的对待边界条件、极端情况,编写边界条件测试。
忽视安全:忽视安全相当危险,比如try..catch很多代码,某次编译警告等。
重复:找到并消除重复,比如switch...case或if...else改为多态。
在错误的抽象层级上的代码:较低抽象层级概念放在派生类,较高层级概念放在基类。与细节实现有关的常量、变量或者工具函数不应当在基类出现。
信息过多:类中方法越少越好,函数知道的变量越少越好,类拥有的实体变量越少越好。隐藏数据、工具函数、常量和临时变量等,不要创建拥有大量方法/实体变量的类,不要为子类创建大量受保护变量和函数。高内聚、低耦合。
死代码:即不执行的代码,找到并体面埋葬他。
垂直分割:变量与函数应该在靠近被使用的地方定义,私有函数应该刚好在首次被使用的位置下面定义。
前后不一致:比如用response来持有HttpServletResponse对象,拿在其他用到HttpServletResponse对象的函数应当用同样的变量名。
混淆视听:实现默认构造器可以优化组织。eg:冷启动阶段执行初始化逻辑。
人为耦合:普通的emun不应该包括在特殊类中,花点时间研究什么地方声明函数、变量和常量。不要随手放置。
特性依恋:类的方法只应对所属类的变量和函数感兴趣,不该垂青其他类中的变量和函数。
//特性依恋:消除特型依恋,因为它将一个类的内部情况暴露给另外一个类。calculateWeeklyPay深入了解了HourlyEmployee。
public class HourlyPayCalculator {
public Money calculateWeeklyPay(HourlyEmployee e) {
int tenthRate = e.getTenthRate().getPennies();
int tenthsWorked = e.getTenthsWorked();
int straightTime = Math.min(400, tenthsWorked);
int overTime = Math.max(0, tenthsWorked - straightTime);
int straightPay = straightTime * tenthRate;
int overtimePay = (int)Math.round(overTime*tenthRate*1.5);
return new Money(straightPay + overtimePay);
}
}
//特型依恋有时不得以而为之,这种情况下特性依恋也可以。
public class HourlyEmployeeReport {
private HourlyEmployee employee ;
public HourlyEmployeeReport(HourlyEmployee e) {
this.employee = e;
}
String reportHours() {
return String.format(
"Name: %s\tHours:%d.%1d\n",
employee.getName(),
employee.getTenthsWorked()/10,
employee.getTenthsWorked()%10);
}
}
选择算子参数:将选择算子参数所在的函数改为多个函数。
//优化前
public int calculateWeeklyPay(boolean overtime) {
int tenthRate = getTenthRate();
int tenthsWorked = getTenthsWorked();
int straightTime = Math.min(400, tenthsWorked);
int overTime = Math.max(0, tenthsWorked - straightTime);
int straightPay = straightTime * tenthRate;
double overtimeRate = overtime ? 1.5 : 1.0 * tenthRate;
int overtimePay = (int)Math.round(overTime*overtimeRate);
return straightPay + overtimePay;
}
//优化后
public int straightPay() {
return getTenthsWorked() * getTenthRate();
}
public int overTimePay() {
int overTimeTenths = Math.max(0, getTenthsWorked() - 400);
int overTimePay = overTimeBonus(overTimeTenths);
return straightPay() + overTimePay;
}
private int overTimeBonus(int overTimeTenths) {
double bonus = 0.5 * getTenthRate() * overTimeTenths;
return (int) Math.round(bonus);
}
晦涩的意图:代码要有表达力,一眼能看出来这玩意干了啥。
//优化前:晦涩难懂
public int m_otCalc() {
return iThsWkd * iThsRte +
(int) Math.round(0.5 * iThsRte *
Math.max(0, iThsWkd - 400)
);
}
位置错误的权责:函数名称决定其放在哪里。比如getTotalHours应当放在时间统计模块中。
不恰当的静态方法:倾向于选用非静态方法,如果有疑问,用非静态函数,如果的确需要静态函数,确保没机会打算让它有多态行为。
new Math().max(a, b)比Math.max(double a, double b)愚蠢,因为max用到的全部数据来自于两个参数而非“所属”对象。
HourlyPayCalculator.calculatePay(employee, overtimeRate)看起来像是个静态函数,并不在特定对象操作&从参数中获取全部数据。但我们可能希望计算每小时支持工资的几种不同算法,比如OvertimeHourlyPayCalculator等,它应该是非静态的。
使用解释性变量:将计算过程打散成有意义的单词变量中放置的中间值。
函数名称应该表达其行为:从函数名称看出函数行为
//优化前
Date newDate = date.add(5);
//优化后:返回五天后日期
Date newDate = date.increaseByDays(5)
理解算法:不敢重构是因为不够理解。
把逻辑依赖改为物理依赖:合适的属性放在合适的位置。
用多态替代if/else或者switch/case。
用命名常量替代魔术数。
准确:代码中的含糊或者不准确要么是意见不统一,要么是懒惰,确保代码是准确的。
封装条件:将解释意图的函数抽离出来。
//重构前:
if (timer.hasExpired() && !timer.isRecurrent())
//重构后:
if (shouldBeDeleted(timer))
避免否定条件:尽可能肯定式。
//重构前
if (!buffer.shouldNotCompact())
//重构后
if (buffer.shouldCompact())
函数只做一件事。
//重构前:做了三件事:遍历所有雇员;检查是否该付工资;支付薪水
public void pay() {
for (Employee e : employees) {
if (e.isPayday()) {
Money pay = e.calculatePay();
e.deliverPay(pay);
}
}
}
//重构后:每个函数只做一件事。
public void pay() {
for (Employee e : employees)
payIfNecessary(e);
}
private void payIfNecessary(Employee e) {
if (e.isPayday())
calculateAndDeliverPay(e);
}
private void calculateAndDeliverPay(Employee e) {
Money pay = e.calculatePay();
e.deliverPay(pay);
}
不应掩蔽时序耦合,显露调用次序。
//重构前:三个函数次序很重要,捕鱼之前先织网,织网之前先编绳。代码没有强制时序耦合,易产生异常,
public class MoogDiver {
Gradient gradient;
List<Spline> splines;
public void dive(String reason) {
saturateGradient();
reticulateSplines();
diveForMoog(reason);
}
...
}
//重构后:每个函数产出下一个函数所需结果。
public class MoogDiver {
public void dive(String reason) {
Gradient gradient = saturateGradient();
List<Spline> splines = reticulateSplines(gradient);
diveForMoog(splines, reason);
}
...
}
封装边界条件:边界条件代码集中一处,不要散落于代码中。
//重构前
if(level + 1 < tags.length) {
parts = new Parse(body, tags, level + 1, offset + endTag);
body = null;
}
//重构后
int nextLevel = level + 1;
if(nextLevel < tags.length) {
parts = new Parse(body, tags, nextLevel, offset + endTag);
body = null;
}
函数应当只在一个抽象层级上:
//重构前:
public String render() throws Exception {
StringBuffer html = new StringBuffer("<hr");
if(size > 0)
html.append(" size=\"").append(size + 1).append("\"");
html.append(">");
return html.toString();
}
//重构后:
public String render() throws Exception {
HtmlTag hr = new HtmlTag("hr");
if (extraDashes > 0)
hr.addAttribute("size", hrSize(extraDashes));
return hr.html();
}
private String hrSize(int height) {
int hrSize = height + 1;
return String.format("%d", hrSize);
}
在较高层级放置可配置数据。
避免传递浏览。
//重构前:
a.getB().getC().doSomething();
//重构后:
a.doSomething();
3.Java和名称
常量和枚举
使用enum而非public static final int。enum学习>>
//优化后:
public class Test5 {
public enum Color {
RED("红色", 1), GREEN("绿色", 2), WHITE("白色", 3), YELLOW("黄色", 4);
private String name;
private int index;
private Color(String name, int index) {
this.name = name;
this.index = index;
}
@Override
public String toString() {
return "Color{" +
"name='" + name + '\'' +
", index=" + index +
'}';
}
}
public static void main(String[] args) {
System.out.println(Color.RED.toString()); // 输出:1-红色
}
}
名称:采用描述性名称,名称应当与抽象层级相符;
//重构前:有可能通过USB、线缆传输,优化策略。
public interface Modem {
boolean dial(String phoneNumber);
boolean disconnect();
boolean send(char c);
char recv();
String getConnectedPhoneNumber();
}
//重构后:名称应当与抽象层级相符。
public interface Modem {
boolean connect(String connectionLocator);
boolean disconnect();
boolean send(char c);
char recv();
String getConnectedLocator();
}
5.测试
测试不足
使用覆盖率工具
别略过小测试,被忽略的测试就是对不确定事物的疑问
测试边界条件
全面测试相近的缺陷:当某个函数发现缺陷,最好全面测试那个函数
测试应当迅速