看了《代码整洁之道》后的一次重构实践

一 前言

这两天重翻了一下 Bob大叔写的《代码整洁之道》。书中最核心的观点就是，代码除了让软件可以工作以外，还有一个重要的任务，就是和人（程序员）进行交流。

另外一本讲如何写好代码、优化代码的书《重构》也是相当值得细品的

写出能和人沟通的代码不容易，通常需要耗费更多的时间、精力。但这些投资（时间、精力）会在日后产生客观的收益，如：

提高软件质量更高 - 整洁清晰的代码，逻辑更清晰，更容易发现问题，减少bug的发生。

衡量代码质量的唯一标准，是别人阅读你代码时的感受

降低开发、运维成本 - Bob大叔曾经做过一个有趣的统计，即便是开发新功能时，90%时间都是花在读代码上，而不是写代码。所以可读性好的代码，可以长效、持续的节省开发、运维成本。

统计方法：使用Emacs进行录屏，发现大部分的时间都再滚屏、文件模块跳转、方法跳转，真正编辑的时间只有10%。

趁着最近不忙，也来优化一个方法 练练手

二 方法业务说明

这里我们拿一个校验行李额是否合法的方法来试试

该方法业务比较简单，核心要求就是 校验用户填写的行李额是符合规范

规范有：

1. 行李额可以是重量单位KG，如20KG
2. 行李额也可以是包裹数量PC，如2PC，代表2个包裹
3. 还有一些比较细节的需求，后续会看到。

现在的代码如下：

检验行李额–主体方法

--（代码片段1）
public static boolean isBaggage(int index, Row row) {
  boolean isExist = true;
  String baggageUnitStr = getValue(row.getCell(index)).toUpperCase();
  if (baggageUnitStr.endsWith("PC") || baggageUnitStr.endsWith("KG")) {
    String baggage = baggageUnitStr.substring(0, (baggageUnitStr.length() - 2));
    if (baggage.indexOf(".") == -1) { //判断是否是小数
      String regEx = "^[0-9]*$";// 判断数字的正则表达式
      isExist = match(regEx, baggage);
    } else {
      String regEx = "^[0-9]+(.[0-9]{0,1})?$";
      isExist = match(regEx, baggage);
    }
    if(isExist){
      if(baggageUnitStr.endsWith("PC")&&Integer.valueOf(baggage) > 5){
        isExist=false;
      }else if(baggageUnitStr.endsWith("KG")&&Integer.valueOf(baggage) > 50){
        isExist=false;
      }
    }
  } else {
    isExist = false;
  }
  return isExist;
}

校验方法一共做了这么几件事情

1. 通过row，colNum获取excel文件中的行李额
2. 通过正则表达式来校验行李额是否合法
3. 校验是否超重

从可读性角度来看，其实还行，毕竟业务并不复杂。但还是有一些问题，让阅读不那么顺畅，譬如：

1. 方法做的三件事（获取行李额、正则校验、重量校验）有点混在一起了，层次不分明
2. 变量名不是那么清晰
   – 如 isExist 如果换成 isPass,isAvailableBaggage更容易理解

检验行李额–消费端代码

--（代码片段2）
// 判断excel中的行李额是否合法
String baggageStr =FileExcelUtil.getValue(row.getCell(7)).toUpperCase(); 
if (!StringUtils.isBlank(baggageStr) && ((!FileExcelUtil.isBaggage(7, row)))) { 
  errorInfos.add("规则设置-第" + (rowNum ) + "行的记录，行李的内容不正确，解析文件失败！");
}

消费端的代码可读性很高。只是行李额的列号出现了2次，不便于后期维护（列号发生变化时，需要修改2处）。
– row.getCell(7) , FileExcelUtil.isBaggage(7, row)

所以比较好的方法是调整一下isBaggage方法的参数，把colIndex，row 调整为 baggageStr
– 参数调整后，不仅可读性更高，也更利于调用。更重要的是 自测不再依赖excel文件了

三 开始重构

做好防御-先编写单元测试

重构之前，最准备一个单元测试，便于后续校验功能是否被破坏。

1. 调整方法入参
– 把isBaggage方法的参数colIndex，row 调整为 baggageStr，并把方法中获取行李额代码去掉

2. 编写测试用例
– 针对业务业务，尽可能覆盖的相对全面一点

3. 跑通所有用例
– 小绿条变绿就ok了
Bugs
跑测试时，还发现原逻辑的2个bug
1.即只有单位PC，KG，没有具体数量，公斤数的情况，程序会报异常。所以就加了一个校验公斤数是否为空的判断，为空时返回校验不通过
2. 具体件数、公斤数填写小数时，也会报错，如20.5KG
– 格式校验的部分可以允许小数，但是到超重校验的地方，只接受整型。咨询业务后，目前没有小数，所以这块代码也要调整。

测试用例的代码如下：

--（代码片段3）
@Test
public void isBaggage() throws Exception {
    FileExcelUtil fileExcelUtil =new FileExcelUtil();
	// 格式正常
    boolean isBaggage= fileExcelUtil.isBaggage("2PC");
    Assert.assertEquals(isBaggage,true);
    isBaggage = fileExcelUtil.isBaggage("20KG");
    Assert.assertEquals(isBaggage,true);
    
	// 格式异常
    isBaggage= fileExcelUtil.isBaggage("20");
    Assert.assertEquals(isBaggage,false);    

    isBaggage = fileExcelUtil.isBaggage("");
    Assert.assertEquals(isBaggage,false);


    isBaggage = fileExcelUtil.isBaggage("ab");
    Assert.assertEquals(isBaggage,false);

    isBaggage = fileExcelUtil.isBaggage("PC");
    Assert.assertEquals(isBaggage,false);

    isBaggage= fileExcelUtil.isBaggage("KG");
    Assert.assertEquals(isBaggage,false);
	
	isBaggage= fileExcelUtil.isBaggage("20.5KG");
    Assert.assertEquals(isBaggage,false); 
    
	// 超重
	isBaggage= fileExcelUtil.isBaggage("6PC");
    Assert.assertEquals(isBaggage,false);
	
    isBaggage= fileExcelUtil.isBaggage("60KG");
    Assert.assertEquals(isBaggage,false);
}

重构主方法

之前为了单元测试时，已经调整了一些地方

1. 修复只有单位PC，KG，没有具体数量，公斤数的Bug
2. 入参改为行李额字符串
3. 修改具体件数、公斤数填写小数的Bug

本节继续重构主方法，主要重构思路如下
用卫语句（guard clauses）简化嵌套逻辑关系
把复杂的条件表达式拆分后，代码的层级变少了
之前的局部变量isExist，现在都不需要了，同样也提高了可读性。

按业务操作进行分块
按业务分块后，即便不了解业务的程序员，也能快速了解方法的业务实现。

重构之后的代码如下：

--（代码片段4）
/**
 * 校验行李信息是否正确
 * @param baggageStr
 * @return
 */
public static boolean isBaggage(String baggageStr) {
 // 1. 是否 ?PC, ?KG 格式
 if (!baggageStr.endsWith("PC") && !baggageStr.endsWith("KG")) {
  return false;
 }

 // 2. 重量值是否合法  
 String baggageQuantity = baggageStr.substring(0, (baggageStr.length() - 2)); 
 if(baggageQuantity.equals("")){
  return false;
 }
 if (baggageQuantity.indexOf(".") == -1) {
  String regEx = "^[0-9]*$";
  if(!match(regEx, baggageQuantity)) {
   return false;
  }
 } else {
  return false;
 }

 // 3. 是否超重
 if(baggageStr.endsWith("PC")&&Integer.valueOf(baggageQuantity) > 5){
  return false;
 }else if(baggageStr.endsWith("KG")&&Integer.valueOf(baggageQuantity) > 50){
  return false;
 }
 return true;
}

继续重构

经过刚刚的重构，发现代码的可读性已经提高了很多。 而且还发现 2.重量值是否合法不就是数值判断嘛，完全可以用已有的公用方法替换掉, 最终的代码如下：

--（代码片段4）
/**
 * 校验行李信息是否正确
 * @param baggageStr
 * @return
 */
public static boolean isBaggage(String baggageStr) {
 // 1. 是否 xPC, xKG 格式
 if (!baggageStr.endsWith("PC") && !baggageStr.endsWith("KG")) {
  return false;
 }

 // 2. 重量值是否合法
 String baggageQuantity = baggageStr.substring(0, (baggageStr.length() - 2));
 if(!MathUtil.isPositiveNum(baggageQuantity)){
  return false;
 }

 // 3. 是否超重
 if(baggageStr.endsWith("PC")&&Integer.valueOf(baggageQuantity) > 5){
  return false;
 }else if(baggageStr.endsWith("KG")&&Integer.valueOf(baggageQuantity) > 50){
  return false;
 }
 return true;
}

四 总结

本次重构的收获如下

1. 重构后，代码的可读性大大提高
2. 重构的确需要花费不少时间、精力
   – 首先需要编写相应的测试用例，其次取一个恰当的方法名、变量名也挺费时间
3. 单元测试+重构 真的是神器
   – 基本可以在不提高编程能力的情况下 ，大大提高软件质量，当然编写合适的测试用例本身也是一种编程能力。
4. 整洁的代码，更容易发现问题，质量也更高的逻辑也是成立的 。