单位转换——重构系列学习笔记

一、题目介绍

这是一道简单的长度单位之间的换算题，代码已经由你的前任程序员写好，但是……实现实在并不怎么优雅，好在前任还算负责，写了很多单元测试。这一课的任务就是对已有的代码做出修改，使它变得更可读，维护性更好。你可以先把代码拉下来，读读README.md里描述的需求，再随意读读代码感受一下，有兴趣的话你也可以尝试上手重构一把。

JavaScript：

1
2
3

git clone https://github.com/linesh-simplicity/length.git
cd length
npm test

二、练习过程

task-01

看着一团糟的代码，确实是日常工作中经常可以见到的代码，迫不及待的上手重构了一下，然而…

parseTo(u) {
    let len = this
    let yard = 'yard'
    let inch = 'inch'
    let f = 'f'

    if (this.unit === yard && u === f) {
        len = new Length(this.value * 3, u)
    }

    if (this.unit === yard && u === inch) {
        len = new Length(this.value * 36, u)
    }

    if (this.unit === inch && u === yard) {
        len = new Length(this.value / 36, u)
    }

    if (this.unit === inch && u === f) {
        len = new Length(this.value / 12, u)
    }

    if (this.unit === f && u === yard) {
        len = new Length(this.value / 3, u)
    }

    if (this.unit === f && u === inch) {
        len = new Length(this.value * 12, u)
    }

    return len
}

除了结构整齐了，还是有明显的坏味道

task-02

这一次，只针对不友好的命名做重构~

conversionTo(targetUnit) {
    let value = this
    if (this.unit === 'yard') {
        if (targetUnit === 'f') {
            value = new Length(this.value * 3, targetUnit)
        } else if (targetUnit === 'inch') {
            value = new Length(this.value * 36, targetUnit)
        }
    }

    if (this.unit === 'inch') {
        if (targetUnit === 'yard') {
            value = new Length(this.value / 36, targetUnit)
        } else if (targetUnit === 'f') {
            value = new Length(this.value / 12, targetUnit)
        }
    }

    if (this.unit === 'f') {
        if (targetUnit === 'yard') {
            value = new Length(this.value / 3, targetUnit)
        } else if (targetUnit === 'inch') {
            value = new Length(this.value * 12, targetUnit)
        }
    }

    return value
}

重构的过程中，随时运行测试，一定牢牢记住小步前进

思考

有没有做到每修改一处代码就运行测试？

答：修改了好几处才想起来, 并没有养成随时运行测试的习惯，应该更频繁的运行测试
你是如何为变量和方法重命名的？全局搜索/替换？使用快捷键？不同的办法之间有什么优劣？

答：最开始用Pycharm ，可能是版本配置的问题，快捷键重命名后代码总是有问题，使用Alt+J 批量替换了一些，之后就安装了webstorm , 批量替换正对字符串，在引用较多的情况下或者有其他变量存在相同字符串的时候，很容易出问题，而依赖Shift+F6 则不容易出现这种问题，更安全

好的命名应该是什么样子？平时工作中的代码元素（变量、函数、类名、常量等）有没有命名不好的例子？

好的命名，简单明了，重点是明了，然后再是简单。遵循规范，可以清晰的表现出用途，同时也相对简洁。

命名不好的例子：

var thLen = $('#maintable thead th').length;
var $trs = $('.m_table tbody tr');
var trLen = $trs.length;
var $fixThs = $('#fixtable th');
var $mainThs = $('#maintable th');
var $fixAs = $('#fixtable th a');

如果你不知道 IDE 对“重命名”这个重构操作有快捷键支持，请你搜索一下，应用到重构中去

这个用的已经比较熟练，日常工作中几乎所有重命名操作都依赖快捷键
重构的节奏顺畅吗？若有不顺畅的地方在哪里？

环境方面遇到一点问题，更换IDE后解决

task-03

重构目标：

重复的字符串提取为常量

重构第一步：

parseTo(unit) {
  let result = this
  const Yard = 'yard';
  const Foot = 'f';
  const Inch = 'inch';

  if (this.unit === Yard) {
    if (unit === Foot) {
      result = new Length(this.value * 3, unit)
    } else if (unit === Inch) {
      result = new Length(this.value * 36, unit)
    }
  }

  if (this.unit === Inch) {
    if (unit === Yard) {
      result = new Length(this.value / 36, unit)
    } else if (unit === Foot) {
      result = new Length(this.value / 12, unit)
    }
  }

  if (this.unit === Foot) {
    if (unit === Yard) {
      result = new Length(this.value / 3, unit)
    } else if (unit === Inch) {
      result = new Length(this.value * 12, unit)
    }
  }

  return result
}

然后，测试代码中也存在很多单位字符串的使用，此时的重构结果很是不让人满意，因此，下一步我创建了一个新的Unit类

export class Unit {
    static Yard = 'yard';
    static Foot = 'f';
    static Inch = 'inch';
}

替换所有用到这些地方的字符串

import { Unit } from './unit'

export class Length {
  value
  unit
  constructor(value, unit) {
    this.value = value
    this.unit = unit
  }

  getValue() {
    return this.value
  }

  getUnit() {
    return this.unit
  }

  parseTo(targetUnit) {
    let result = this

    if (this.unit === Unit.Yard) {
      if (targetUnit === Unit.Foot) {
        result = new Length(this.value * 3, targetUnit)
      } else if (targetUnit === Unit.Inch) {
        result = new Length(this.value * 36, targetUnit)
      }
    }

    if (this.unit === Unit.Inch) {
      if (targetUnit === Unit.Yard) {
        result = new Length(this.value / 36, targetUnit)
      } else if (targetUnit === Unit.Foot) {
        result = new Length(this.value / 12, targetUnit)
      }
    }

    if (this.unit === Unit.Foot) {
      if (targetUnit === Unit.Yard) {
        result = new Length(this.value / 3, targetUnit)
      } else if (targetUnit === Unit.Inch) {
        result = new Length(this.value * 12, targetUnit)
      }
    }

    return result
  }
}

包括测试代码中的

import { Length } from './index'
import {Unit} from "./unit"

describe('Length', () => {
  it("should 1 'foot' equals 1 'foot'", () => {
    const length = new Length(1, Unit.Foot)

    expect(length.getValue()).toEqual(1)
    expect(length.getUnit()).toEqual(Unit.Foot)
  })

  it("should 1 'foot' equals 12 inches", () => {
    const result = new Length(1, Unit.Foot).parseTo(Unit.Inch)

    expect(result.getValue()).toEqual(12)
    expect(result.getUnit()).toEqual(Unit.Inch)
  })

  it("should 3 feet equals 1 'yard'", () => {
    const result = new Length(3, Unit.Foot).parseTo(Unit.Yard)

    expect(result.getValue()).toEqual(1)
    expect(result.getUnit()).toEqual(Unit.Yard)
  })

  it("should 1 'yard' equals 3 feet", () => {
    const result = new Length(1, Unit.Yard).parseTo(Unit.Foot)

    expect(result.getValue()).toEqual(3)
    expect(result.getUnit()).toEqual(Unit.Foot)
  })

  it("should 1 'yard' equals 36 inches", () => {
    const result = new Length(1, Unit.Yard).parseTo(Unit.Inch)

    expect(result.getValue()).toEqual(36)
    expect(result.getUnit()).toEqual(Unit.Inch)
  })

  it("should 1 'yard' equals 1 'yard'", () => {
    const result = new Length(1, Unit.Yard).parseTo(Unit.Yard)

    expect(result.getValue()).toEqual(1)
    expect(result.getUnit()).toEqual(Unit.Yard)
  })

  it("should 12 inches equals 1 'foot'", () => {
    const result = new Length(12, Unit.Inch).parseTo(Unit.Foot)

    expect(result.getValue()).toEqual(1)
    expect(result.getUnit()).toEqual(Unit.Foot)
  })

  it("should 36 inches equals 1 'yard'", () => {
    const result = new Length(36, Unit.Inch).parseTo(Unit.Yard)

    expect(result.getValue()).toEqual(1)
    expect(result.getUnit()).toEqual(Unit.Yard)
  })

  it("should 1 inch equals 1 'inch'", () => {
    const result = new Length(1, Unit.Inch).parseTo(Unit.Inch)

    expect(result.getValue()).toEqual(1)
    expect(result.getUnit()).toEqual(Unit.Inch)
  })
})

最后，将f干掉

export class Unit {
    static Yard = 'yard';
    static Foot = 'foot';
    static Inch = 'inch';
}

task-4

在第3步就已经完成这这一步需要做的操作，可能步子还是有点大

总结一下吧

思考：

你还能做到重构过程每分钟都保持运行测试并且测试通过的状态吗？

这次测试运行的很频繁，好几次出现的测试运行失败，立马先恢复，这种感觉很安全
过程中有没有不顺畅的地方？

对JS面向对象的语法不够熟悉，看了下task-4才知道static关键字
你选择先更换哪个方法的声明？你觉得重构的先后次序有区别吗？

重构的先后次序对结果的影响可能不大，但不同的次序重构的难度是不一样的

task-5

练习三遍

第一遍使用快捷键加批量替换，有些时候，重构快捷键并不是那么智能，可以配合替换快捷键使用提高效率

第二遍使用纯手工替换每一个变量，每替换一个，就运行一次测试，很有安全感

第三遍再会到快捷键操作，还是快捷键效率高

思考：

注意体会“旧的不变、新的创建”这个创建重构中间状态的过程
重构的过程有没有更加顺畅？
重构的次序能不能进一步优化？

task-6

这一节的任务，是提炼计算函数

动手之前，先花几分钟思考一下重构步骤：如何做到平滑替换？

旧的不变，新的创建，一步切换，旧的再见

严格按照重构步骤执行重构，频繁运行测试
git checkout final看一下参考步骤，找出与你设定步骤的异同
多练几遍

第一步：

创建一个新的函数

1
2
3

getLength(value, unit) {
    return new Length(value, unit);
}

使用这个函数替换 new Length()的调用

parseTo(unit) {
    let result = this
    if (this.unit === Unit.YARD) {
        if (unit === Unit.FOOT) {
            result = this.getLength(this.value * 3, unit)
        } else if (unit === Unit.INCH) {
            result = this.getLength(this.value * 36, unit)
        }
    }

    if (this.unit === Unit.INCH) {
        if (unit === Unit.YARD) {
            result = this.getLength(this.value / 36, unit)
        } else if (unit === Unit.FOOT) {
            result = this.getLength(this.value / 12, unit)
        }
    }

    if (this.unit === Unit.FOOT) {
        if (unit === Unit.YARD) {
            result = this.getLength(this.value / 3, unit)
        } else if (unit === Unit.INCH) {
            result = this.getLength(this.value * 12, unit)
        }
    }

    return result
}

然后，消除嵌套的if表达式

parseTo(unit) {
    let result = this
    if (this.unit === Unit.YARD && unit === Unit.FOOT) {
        result = this.getLength(this.value * 3, unit)
    }
    if (this.unit === Unit.YARD && unit === Unit.INCH) {
        result = this.getLength(this.value * 36, unit)
    }
    if (this.unit === Unit.INCH && unit === Unit.YARD) {
        result = this.getLength(this.value / 36, unit)
    }
    if (this.unit === Unit.INCH && unit === Unit.FOOT) {
        result = this.getLength(this.value / 12, unit)
    }
    if (this.unit === Unit.FOOT && unit === Unit.YARD) {
        result = this.getLength(this.value / 3, unit)
    }
    if (this.unit === Unit.FOOT && unit === Unit.INCH) {
        result = this.getLength(this.value * 12, unit)
    }
    return result
}

殊途同归，又回到了第一次重构时的结构，但总觉的有些不对，看看示例，有一个坏味道被我忽略了

示例代码：


as(targetUnit) {
  return new Length(this._valueIn(targetUnit), targetUnit)
}

_valueIn(unit) {
  if (this.unit === Unit.YARD) {
    if (unit === Unit.FOOT) {
      return this.value * 3
    }

    if (unit === Unit.INCH) {
      return this.value * 36
    }
  }

  if (this.unit === Unit.INCH) {
    if (unit === Unit.YARD) {
      return this.value / 36
    }
    if (unit === Unit.FOOT) {
      return this.value / 12
    }
  }

  if (this.unit === Unit.FOOT) {
    if (unit === Unit.YARD) {
      return this.value / 3
    }
    if (unit === Unit.INCH) {
      return this.value * 12
    }
  }

  return this.value
}

重复的this.getLength() 和重复的 new Length()真的有很大的区别吗？

事实证明，没有仔细审题就动手的后果很严重！

仔细观察，其实Length对象的创建过程，唯一区别是在计算不同的value值。不过，要消除重复的对象创建代码，我们还得做点准备工作，那就是把这部分关于value的计算逻辑先剥离出来。

这次的任务是提炼计算函数，计算函数就是计算value的逻辑，如何平滑切换呢？

十六字心法背的很熟练，旧的不变，新的创建，先创建什么呢？

先创建一个新的变量_value代表每次计算逻辑计算后的值，需要注意的是，parseTo函数，即时传入的unit不能满足所有的条件，也是有返回的，这种情况下返回的就是当前对象本身，因此计算出的value实际就是this.value

1	let _value = this.value