菜鸟笔记
提升您的技术认知

代码整洁之道(下篇)

目录

前言

并发编程

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.测试
测试不足
使用覆盖率工具
别略过小测试,被忽略的测试就是对不确定事物的疑问
测试边界条件
全面测试相近的缺陷:当某个函数发现缺陷,最好全面测试那个函数
测试应当迅速