BT

如何利用碎片时间提升技术认知与能力? 点击获取答案

专栏:代码之丑(四)——代码找茬游戏

| 作者 郑晔 关注 2 他的粉丝 发布于 2010年11月26日. 估计阅读时间: 3 分钟 | Google、Facebook、Pinterest、阿里、腾讯 等顶尖技术团队的上百个可供参考的架构实例!

这是一个找茬的游戏,下面三段代码的差别在哪:

if (1 == insertFlag) {
    retList.insert(i, newCatalog);
  } else {
    retList.add(newCatalog);
  }
if (1 == insertFlag) {
    retList.insert(m, newCatalog);
  } else {
    retList.add(newCatalog);
  }
if (1 == insertFlag) {
    retList.insert(j, newPrivNode);
  } else {
    retList.add(newPrivNode);
  }

答案时间:除了用到变量之外,完全相同。我想说的是,这是我从一个文件的一次diff中看到的。

不妨设想一下修改这些代码时的情形:费尽九牛二虎之力,我终于找到该在哪改动代码,然后改了。作为一个有职业操守的程序员,我知道别的地方也需要类似的修改。 于是,趁人不备,把刚做修改拷贝了一份,放到另外需要修改的地方。修改了几个变量,编译通过了。世界应该就此清净,至少问题解决了。

好吧!虽然这个程序员有职业操守的程序员,却缺少了些职业技能,至少在挥舞着“拷贝粘贴”的锤子时,他没有嗅到散发出的臭味。

只要意识到坏味道,修改是件很容易的事,提出一个新函数即可:

void AddNode(List& retList, int insertFlag, int pos, Node& node) {
    if (1 == insertFlag)    {
      retList.insert(pos, Node);
    } else {
      retList.add(node);
    }
  }

于是,原来那三段代码变成了三个调用:

AddNode(retList, insertFlag, i, newCatalog);
AddNode(retList, insertFlag, m, newCatalog);
AddNode(retList, insertFlag, j, newPrivNode);

当然,这种修改只是一个局部的微调,如果有更多的上下文信息,我们可以做得更好。

重复,是最为常见的坏味道。上面这种重复实际上是非常容易发现的,也是很容易修改。但所有这一切的前提是,发现坏味道。长时间生活在这种代码里面,我们会对坏味道失去嗅觉。更可怕的是,一个初来乍到的嗅觉尚灵敏的人意识到这个问题,那些失去嗅觉的人却告诫他,别乱动,这挺好。

趁嗅觉尚在,请坚持代码正义。

作者简介:

郑晔,ThoughtWorks公司咨询师,拥有多年企业级软件开发经验,热衷于探索各种程序设计语言在真实软件开发中所能发挥的威力,致力于探寻合理的软件开发方式,加入ThoughtWorks公司后,投入到敏捷开发方法的实践之中,为其他公司提供敏捷开发方法方面的咨询服务。他的blog是梦想风暴

查看原文:代码之丑(四)

评价本文

专业度
风格

您好,朋友!

您需要 注册一个InfoQ账号 或者 才能进行评论。在您完成注册后还需要进行一些设置。

获得来自InfoQ的更多体验。

告诉我们您的想法

允许的HTML标签: a,b,br,blockquote,i,li,pre,u,ul,p

当有人回复此评论时请E-mail通知我

DRY原则 by 侯 伯薇

这就是传说中的DRY(don't repeat yourself)原则吧,哈哈。

我个人觉得,一个函数必须的参数如果超过三个,那就很难读了 by 冯 希顺

我个人觉得,一个函数必须的参数如果超过三个,那就很难读了。

这是作者改写后的代码:


void AddNode(List& retList, int insertFlag, int pos, Node& node) {
if (1 == insertFlag) {
retList.insert(pos, Node);
} else {
retList.add(node);
}
}





如果不怕麻烦,还可以改写成这样:


private class ListWithSpecialAddRule extends List {
int insertFlag;

public PositionAddList(int insertFlag) {
this.insertFlag = insertFlag;
}

public void add(int pos, Node& node) {
if (1 != insertFlag) {
pos = this.size();
}

this.insert(pos, Node);
}
}

Re: 我个人觉得,一个函数必须的参数如果超过三个,那就很难读了 by 黄 雯

同意 参数大于3个 应尽量将参数封装

还有个问题请教编辑 by 黄 雯

网站时间为什么是 格林威治时间 不能改成北京时间吗

Re: 我个人觉得,一个函数必须的参数如果超过三个,那就很难读了 by see sai

win32 api都有很多个。。

Re: 我个人觉得,一个函数必须的参数如果超过三个,那就很难读了 by 张 核铭

难道win32 api很好读?

说实话,代码越改越丑了~~~ by 高 翌翔

原来的代码只说明了“怎么做”,至于代码的目的是什么(即“做什么”)要读者自己领悟。

修改后的代码仅仅是一种形式化的转换!

封装的方法 AddNode 仅仅是一种对代码本身的注释,而并没有从业务角度说明“做什么”;
此外,AddNode 的功能并不单一,其中包含了插入或添加的选择逻辑,而方法名却只反应了添加功能,
这使得方法名变得含混不清,不能一目了然。

总之,这种机械地 Extract Method,不值得提倡!

方法命名是一件非常慎重而严肃的工作,信手拈来的方法名所散发的臭味比重复代码更令人作呕!

在尚未找到恰当的方法名之前,我宁愿保留重复代码!

Re: 机械地形式转换不值得提倡! by 高 翌翔

to xishun feng

首先,为了缩短参数而使用继承实在太草率;
其次,一个方法拆分为两个紧密耦合的方法,只是增加了复杂度而已,费力不讨好!

长参数确实不值得提倡,但绝不能通过机械地拆分来缩短参数!

Re: 说实话,代码越改越丑了~~~ by 毕 成栋

已经不错了,这是我每期看到现在,唯一“找不出茬”的文章。
不过method name确实臭了点。可以改一个addProjNode或者addBomNode之类的名字。
另外if (1 == insertFlag) 这种“魔术数字”也可以去掉。

如何信服 by 廖 锋

lz意欲追求完美,如何让读者认同这种美呢?
或许每篇文章只关注重构的一小领域
但所有的重构都可以运用到整个示例代码中
如果示例代码恰恰出现了Bad Smell,这种认同感就大打折扣了
所以,斟酌示例代码~

Re: DRY原则 by jian lv

能把缩写的全称写出来,很支持。。。 经常泡论坛见很多人 大片大片的缩写,而且也不提供相关解释的链接,不了解的人根本看不懂....

看了这篇感觉,自己代码里好像有很多是这样的。。
进度赶的紧的时候,重构的事情基本抛到脑后了.. 以后得多注意注意。

Re: 我个人觉得,一个函数必须的参数如果超过三个,那就很难读了 by chen Joseph

不认为为了函数参数数量而去封装成类,这样可读性差了,而且复杂度增加。因为你得考虑组合或者继承,而且增加的代码量,应用程序也变大了,为了一个简单的function,最终甚至影响到应用程序大小或者性能,得不偿失。
函数参数数量不能说明可读性,可能好点的去掉多参数的方式是其实是把上文中的pos和node做成一个参数,pair<pos, node>就可以, 因为pos和node其实是依赖性参数,另外List& retList不作为参数传递(当然这个要依赖于上下文),如:

List& retList;
typedef pair<int, Node&> NodePair;

void AddNode(int insertFlag, NodePair &np)
{
if (1 == insertFlag) {
retList.insert(np.first, np.second);
} else {
retList.add(np.second);
}
}</int,></pos,>

些这代码的员工招进来应该先培训下 by Shichao Liu

如题,我觉得这帖子不算技术探讨,倒像是8挂娱乐。

我觉得这样更好 by lei chen

写2个方法:
1,AddNode(...)
2,InsertNode(...)
然后去调用,这样方法这则单一,便于以后修改,而且省去if语句,代码清晰度更高

retList……似乎不应该是个List by Peng Shawn

我们认为List(列表)用来存储一组类似的元素,是相同业务模型的不同实例的集合。我感觉retList不是一个面向对象的设计,因为从这段程序看起来retList的每个位置都存放着不同业务含义的对象,如果它能发生一些变化程序可能完全不是这个样子。

允许的HTML标签: a,b,br,blockquote,i,li,pre,u,ul,p

当有人回复此评论时请E-mail通知我

允许的HTML标签: a,b,br,blockquote,i,li,pre,u,ul,p

当有人回复此评论时请E-mail通知我

15 讨论

登陆InfoQ,与你最关心的话题互动。


找回密码....

Follow

关注你最喜爱的话题和作者

快速浏览网站内你所感兴趣话题的精选内容。

Like

内容自由定制

选择想要阅读的主题和喜爱的作者定制自己的新闻源。

Notifications

获取更新

设置通知机制以获取内容更新对您而言是否重要

BT