2007-02-16
DRY与简单性的平衡
这个事例说起来相当简单。不过小中见大,它大致体现了我和pair在DRY vs. 简单性上的差别,和那个“这样代码重用”里面的例子体现了同样的分歧。
目标是重构下面的测试代码:
首先通过发现重复,我和pair都同意下面的重构:
分歧来自于下一步,我认为重构到下面这样就够了:
而pair重构到这一步后,仍然觉得不够DRY,坚决推进下一步的重构:
我的观点:
最后这一步的重构,是以简单性换取非常细微的DRY,并不划算。最好的情况,也就是和重构前半斤八两,所以不值得投入时间来做。而重构前的代码更能灵活适应变化。
比如,经过分析,我们完全可以仅assertEquals(2, arr.length)而不用分别对每个元素进行assert了(同样的逻辑在别的测试中已经覆盖了,而且,测试代码决定"type1", "type2"的顺序也加大了测试和被测试代码的耦合,这个顺序本来是无所谓的。)这个变化在重构前很容易,只要从testNull()里面删掉两行assertEquals就可。而重构后的代码则需要更复杂的逻辑控制才能达到这个目标。
那么,你怎么看?
目标是重构下面的测试代码:
public void test1() {
Account acct = new Account();
acct.setName("test");
acct.setType(TypeEnum.Type1);
acct.setActive(true);
Result result = runSomeApi(acct);
assertEquals("test", result.getName());
assertTrue(result.isActive());
ResultType[] arr = result.getTypes();
assertEquals(1, arr.length);
assertEquals("type1", arr[0].getName());
}
public void test2() {
Account acct = new Account();
acct.setName("test");
acct.setType(TypeEnum.Type2);
acct.setActive(true);
Result result = runSomeApi(acct);
assertEquals("test", result.getName());
assertTrue(result.isActive());
ResultType[] arr = result.getTypes();
assertEquals(1, arr.length);
assertEquals("type2", arr[0].getName());
}
public void testNull(){
Account acct = new Account();
acct.setName("test");
acct.setType(null);
acct.setActive(true);
Result result = runSomeApi(acct);
assertEquals("test", result.getName());
assertTrue(result.isActive());
ResultType[] arr = result.getTypes();
assertEquals(2, arr.length);
assertEquals("type1", arr[0].getName());
assertEquals("type2", arr[1].getName());
}
首先通过发现重复,我和pair都同意下面的重构:
private String name = "test";
private getResult(TypeEnum type) {
Account acct = new Account();
acct.setName(name);
acct.setType(type);
acct.setActive(true);
return runSomeApi(acct);
}
public void test1() {
Result result = getResult(TypeEnum.Type1);
assertEquals(name, result.getName());
assertTrue(result.isActive());
ResultType[] arr = result.getTypes();
assertEquals(1, arr.length);
assertEquals("type1", arr[0].getName());
}
public void test2() {
Result result = getResult(TypeEnum.Type2);
assertEquals("test", result.getName());
assertTrue(result.isActive());
ResultType[] arr = result.getTypes();
assertEquals(1, arr.length);
assertEquals("type2", arr[0].getName());
}
public void testNull(){
Result result = getResult(null);
assertEquals(name, result.getName());
assertTrue(result.isActive());
ResultType[] arr = result.getTypes();
assertEquals(2, arr.length);
assertEquals("type1", arr[0].getName());
assertEquals("type2", arr[1].getName());
}
分歧来自于下一步,我认为重构到下面这样就够了:
private String name = "test";
private getResult(TypeEnum type) {
Account acct = new Account();
acct.setName(name);
acct.setType(type);
acct.setActive(true);
return runSomeApi(acct);
}
private void assertInvariants(Result result) {
assertEquals("test", result.getName());
assertTrue(result.isActive());
}
private void verifyWhenTypeIsNotNull(String expectedType, TypeEnum type){
Result result = getResult(type);
assertInvariants(result);
ResultType[] arr = result.getTypes();
assertEquals(1, arr.length);
assertEquals(expectedType, arr[0].getName());
}
public void test1() {
verifyWhenTypeIsNotNull("type1" TypeEnum.Type1);
}
public void test2() {
verifyWhenTypeIsNotNull("type2", TypeEnum.Type2);
}
public void testNull(){
Result result = getResult(null);
assertInvariants(result);
ResultType[] arr = result.getTypes();
assertEquals(2, arr.length);
assertEquals("type1", arr[0].getName());
assertEquals("type2", arr[1].getName());
}
而pair重构到这一步后,仍然觉得不够DRY,坚决推进下一步的重构:
private String name = "test";
private getResult(TypeEnum type) {
Account acct = new Account();
acct.setName(name);
acct.setType(type);
acct.setActive(true);
return runSomeApi(acct);
}
private void verify(String[] expectedTypes, TypeEnum type){
Result result = getResult(type);
assertEquals("test", result.getName());
assertTrue(result.isActive());
ResultType[] arr = result.getTypes();
assertEquals(expectedTypes.length, arr.length);
for(int i=0; i<expectedTypes.length; i++) {
assertEquals(expectedTypes[i], arr[i]);
}
}
public void test1() {
verify(new String[]{"type1"} TypeEnum.Type1);
}
public void test2() {
verify(new String[]{"type2"}, TypeEnum.Type2);
}
public void testNull(){
verify(new String[]{"type1", "type2"}, null);
}
我的观点:
最后这一步的重构,是以简单性换取非常细微的DRY,并不划算。最好的情况,也就是和重构前半斤八两,所以不值得投入时间来做。而重构前的代码更能灵活适应变化。
比如,经过分析,我们完全可以仅assertEquals(2, arr.length)而不用分别对每个元素进行assert了(同样的逻辑在别的测试中已经覆盖了,而且,测试代码决定"type1", "type2"的顺序也加大了测试和被测试代码的耦合,这个顺序本来是无所谓的。)这个变化在重构前很容易,只要从testNull()里面删掉两行assertEquals就可。而重构后的代码则需要更复杂的逻辑控制才能达到这个目标。
那么,你怎么看?
- 18:45
- 浏览 (3992)
- 评论 (2)
- 分类: I disagree
- 进入论坛
- 相关推荐
评论
fixopen
2007-08-15
我同意gigix,DRY是指对一件事情说并且只说一次。
程序里面如果发现某些代码是重复的,那么应该区分出来是两件事情还是一件事情,如果因为某些原因,这两件事情看起来似乎很像,但逻辑上仍然是两件事情,不要试图去消除这种重复,除非能在更高层次把这两件事情归并为一件事情。
程序里面如果发现某些代码是重复的,那么应该区分出来是两件事情还是一件事情,如果因为某些原因,这两件事情看起来似乎很像,但逻辑上仍然是两件事情,不要试图去消除这种重复,除非能在更高层次把这两件事情归并为一件事情。
gigix
2007-02-16
在我看来,问题的关键就在于“verify”这个名字。你抽取一个方法却发现很难给它起一个更漂亮的名字,这本身就说明这些代码只是偶然地类似,而不是有什么内在的业务逻辑。强行抽取这种偶然类似的代码,是对模块内聚性的破坏。这一步的调整不是重构,因为它没有让软件的质量提升。
所以我的建议是等一等:等到重复代码真正出现的时候、给未来的添加代码或者修改代码带来困扰的时候再重构。例如你们的这个例子,最后一步重构的动机实际上是想象出来的,它并没有带来任何困扰。
所以我的建议是等一等:等到重复代码真正出现的时候、给未来的添加代码或者修改代码带来困扰的时候再重构。例如你们的这个例子,最后一步重构的动机实际上是想象出来的,它并没有带来任何困扰。
- 浏览: 182189 次

- 详细资料
搜索本博客
最近加入圈子
最新评论
-
SQL 小技巧
第三个问题,先写出代码来吧。等有点时间再解释一下。第四个问题其实可以照猫画虎的: ...
-- by ajoo -
SQL 小技巧
第一个问题是我在维护一个金融分析软件的时候碰到的。原来的那位老兄正儿八经地用一个 ...
-- by ajoo -
SQL 小技巧
效率没问题。实际上一般的query效率都在查询上,至于对查询结果的计算,代价基本 ...
-- by ajoo -
Not Convinced about Java ...
最讨厌所谓的魔法了,调试的时候能让人吐血。
-- by aninfeel -
SQL 小技巧
ajoo 写道Readonly 写道问题一,经过google得到一用sum,lo ...
-- by Readonly






评论排行榜