Conversation
EthanLin-TWer
left a comment
EthanLin-TWer
left a comment
There was a problem hiding this comment.
嗯，这个手法很重要：在改变方法签名前，先把它提炼成一个temp_XXX()的方法，以便新旧共存。
提炼方法后随手修改不表意的方法参数也是好习惯。
| * Date: 2020/2/1 | ||
| * Description: | ||
| **/ | ||
| public enum Unit { |
There was a problem hiding this comment.
这个模板可以配IDE让它不打出来的，这些信息从版本管理里面都能看到。
| public void should_1_inch_equals_1_inch() { | ||
| Length result = new Length(1, Length.INCH).as(Length.INCH); | ||
| Length result = new Length(1, Length.INCH).temp_as(Length.INCH, Unit.INCH); | ||
|
|
There was a problem hiding this comment.
在这个 27deb25 提交里，对temp_as的引用替换早了。
现在temp_as(String, Unit)的方法签名还不是你期望的那样temp_as(Unit)，没必要过早替换，否则后面改过来了还得再替换一波，浪费时间。这个地方还是保持as()方法调用就好，这也是你做中间层的价值：老的引用点仍然能工作。
看了你后面的提交，发觉其实你这么做也可以，挺好。就是剩下那个String类型的参数删掉以后可以让IDE帮你去掉，不需要自己再替换一波方法签名，那我前面说的问题就不存在。
src/main/java/Length.java
Outdated
| } else if (targetUnit.equals(Length.INCH)) { | ||
| tempUnit = Unit.INCH; | ||
| } | ||
| return temp_as(targetUnit, tempUnit); |
There was a problem hiding this comment.
这次替换是一次完成的吗？做的过程能不能保持每做一两行修改就能运行通过测试呢？可以的话步子就没问题。
如果你步子比较大的一点，甚至可以尝试这样来做中间层：
public Length as(String targetUnit) {
Unit tempUnit = Unit.valueOf(targetUnit.toUpperCase());
return temp_as(targetUnit, tempUnit);
}不过，不管步子大小都应该能做到每改动一两行代码就能运行测试并且通过。
|
|
||
| public Length as(String targetUnit, Unit tempUnit) { | ||
| public Length as(Unit unit) { | ||
| Length len = this; |
There was a problem hiding this comment.
这里删除String targetUnit参数是用IDE删的吗？是的话就很标准做法，不然自己一步删步子就太大了哟。
|
总体看来对“旧的不变，新的创建”的应用和节奏还是很对的哈。提交的粒度也很合适👍加中间层、替换引用点、删除老方法、重命名新方法/参数，各自分别是一件事，我review起来能明白你的重构过程。 |
|
谢谢大佬抽出时间点评，很多步骤用了ide的 ^+G 快捷键全选替换，所以步子显得有些大，不过测试通过和提交之间的时间间隔都是严格控制了的哈。 |
|
好，那在替换引用点这件事上你应该已经比较熟练了，很棒哟。 |
2 participants
No description provided.