把函数写短,越短越好
坏味道排查列表
- 长函数
解决方案
- 定义好函数长度的标准
- 做好“分离关注点”
- 坚守“童子军规(让营地比你来的时候更干净)”
详细说明
多长的函数算长呢?
我们对于函数长度容忍度高,这是导致长函数产生的关键点。
如果一个人认为 100 行代码不算长,那在他眼中,很多代码根本就是没有问题的,也就更谈不上看到更多问题了,这其实是一个观察尺度的问题。这就好比,没有电子显微镜之前,人们很难理解疾病的原理,因为看不到病毒,就不可能理解病毒可以致病这个道理。 一个好的程序员面对代码库时要有不同尺度的观察能力,看设计时,要能够高屋建瓴,看代码时,要能细致入微。
随着对代码长度容忍度的降低,对代码细节的感知力就会逐渐提升,你才能看到那些原本所谓细枝末节的地方隐藏的各种问题。
“越小越好”是一个追求的目标,不过,没有一个具体的数字,就没办法约束所有人的行为。所以,通常情况下,我们还是要定义出一个代码行数的上限,以保证所有人都可以按照这个标准执行。
一个比较宽松的标准就是一个方法限制最长20行
在 Java 中,我们就可以把代码行的约束加到 CheckStyle 的配置文件中,就像下面这样:
<module name="MethodLength">
<property name="tokens" value="METHOD_DEF"/>
<property name="max" value="20"/>
<property name="countEmpty" value="false"/>
</module>
这样,在我们提交代码之前,执行本地的构建脚本,就可以把长函数检测出来
长函数的产生
长函数的产生有如下几点原因:
- 性能为由
- 平铺直叙
- 一次加一点
以性能能为由
在一些写汇编语言的人看来,调用函数涉及到入栈出栈的过程,显然不如直接执行来得性能高。这种想法经过各种演变流传到今天。
所以,在很多人看来,把函数写长是为了所谓性能。不过,这个观点在今天是站不住的。性能优化不应该是写代码的第一考量。
一方面,一门有活力的程序设计语言本身是不断优化的,无论是编译器,还是运行时,性能都会越来越好;另一方面,可维护性比性能优化要优先考虑,当性能不足以满足需要时,我们再来做相应的测量,找到焦点,进行特定的优化。这比在写代码时就考虑所谓性能要更能锁定焦点,优化才是有意义的。
平铺直叙
一种最常见的原因也会把代码写长,那就是写代码平铺直叙,把自己想到的一点点罗列出来。
比如如下的代码:
public void executeTask() {
ObjectMapper mapper = new ObjectMapper();
CloseableHttpClient client = HttpClients.createDefault();
List<Chapter> chapters = this.chapterService.getUntranslatedChapters();
for (Chapter chapter : chapters) {
// Send Chapter
SendChapterRequest sendChapterRequest = new SendChapterRequest();
sendChapterRequest.setTitle(chapter.getTitle());
sendChapterRequest.setContent(chapter.getContent());
HttpPost sendChapterPost = new HttpPost(sendChapterUrl);
CloseableHttpResponse sendChapterHttpResponse = null;
String chapterId = null;
try {
String sendChapterRequestText = mapper.writeValueAsString(sendChapterRequest);
sendChapterPost.setEntity(new StringEntity(sendChapterRequestText));
sendChapterHttpResponse = client.execute(sendChapterPost);
HttpEntity sendChapterEntity = sendChapterPost.getEntity();
SendChapterResponse sendChapterResponse = mapper.readValue(sendChapterEntity.getContent(), SendChapterResponse.class);
chapterId = sendChapterResponse.getChapterId();
} catch (IOException e) {
throw new RuntimeException(e);
} finally {
try {
if (sendChapterHttpResponse != null) {
sendChapterHttpResponse.close();
}
} catch (IOException e) {
// ignore
}
}
// Translate Chapter
HttpPost translateChapterPost = new HttpPost(translateChapterUrl);
CloseableHttpResponse translateChapterHttpResponse = null;
try {
TranslateChapterRequest translateChapterRequest = new TranslateChapterRequest();
translateChapterRequest.setChapterId(chapterId);
String translateChapterRequestText = mapper.writeValueAsString(translateChapterRequest);
translateChapterPost.setEntity(new StringEntity(translateChapterRequestText));
translateChapterHttpResponse = client.execute(translateChapterPost);
HttpEntity translateChapterEntity = translateChapterHttpResponse.getEntity();
TranslateChapterResponse translateChapterResponse = mapper.readValue(translateChapterEntity.getContent(), TranslateChapterResponse.class);
if (!translateChapterResponse.isSuccess()) {
logger.warn("Fail to start translate: {}", chapterId);
}
} catch (IOException e) {
throw new RuntimeException(e);
} finally {
if (translateChapterHttpResponse != null) {
try {
translateChapterHttpResponse.close();
} catch (IOException e) {
// ignore
}
}
}
}
这段代码的逻辑是,把没有翻译过的章节发到翻译引擎,然后,启动翻译过程。在这里翻译引擎是另外一个服务,需要通过 HTTP 的形式向它发送请求。相对而言,这段代码还算直白。
这段代码之所以很长,主要原因就是把前面所说的逻辑全部平铺直叙地摆在那里了,这里既有业务处理的逻辑,比如,把章节发送给翻译引擎,然后,启动翻译过程;又有处理的细节,比如,把对象转成 JSON,然后,通过 HTTP 客户端发送出去。
我们可以看到平铺直叙的代码存在的两个典型问题:
- 把多个业务处理流程放在一个函数里实现;
- 把不同层面的细节放到一个函数里实现。
这里发送章节和启动翻译是两个过程,显然,这是可以放到两个不同的函数中去实现的,所以,我们只要做一下提取函数,就可以把这个看似庞大的函数拆开,而拆出来的几个函数规模都会小很多,像下面这样:
public void executeTask() {
ObjectMapper mapper = new ObjectMapper();
CloseableHttpClient client = HttpClients.createDefault();
List<Chapter> chapters = this.chapterService.getUntranslatedChapters();
for (Chapter chapter : chapters) {
String chapterId = sendChapter(mapper, client, chapter);
translateChapter(mapper, client, chapterId);
}
}
拆出来的部分,实际上就是把对象打包发送的过程,我们以发送章节为例,拆出来的发送章节部分:
private String sendChapter(final ObjectMapper mapper,
final CloseableHttpClient client,
final Chapter chapter) {
SendChapterRequest request = asSendChapterRequest(chapter);
CloseableHttpResponse response = null;
String chapterId = null;
try {
HttpPost post = sendChapterRequest(mapper, request);
response = client.execute(post);
chapterId = asChapterId(mapper, post);
} catch (IOException e) {
throw new RuntimeException(e);
} finally {
try {
if (response != null) {
response.close();
}
} catch (IOException e) {
// ignore
}
}
return chapterId;
}
private HttpPost sendChapterRequest(final ObjectMapper mapper, final SendChapterRequest sendChapterRequest) throws JsonProcessingException, UnsupportedEncodingException {
HttpPost post = new HttpPost(sendChapterUrl);
String requestText = mapper.writeValueAsString(sendChapterRequest);
post.setEntity(new StringEntity(requestText));
return post;
}
private String asChapterId(final ObjectMapper mapper, final HttpPost sendChapterPost) throws IOException {
String chapterId;
HttpEntity entity = sendChapterPost.getEntity();
SendChapterResponse response = mapper.readValue(entity.getContent(), SendChapterResponse.class);
chapterId = response.getChapterId();
return chapterId;
}
private SendChapterRequest asSendChapterRequest(final Chapter chapter) {
SendChapterRequest request = new SendChapterRequest();
request.setTitle(chapter.getTitle());
request.setContent(chapter.getContent());
return request
}
长函数还有一个比价隐晦的弊端,就是命名的问题,因为函数长,可能会导致命名的冲突。而当函数精炼出来之后很多的变量的命名都在一个提取的函数的上下文中了,就不会发生命名冲突了。
平铺直叙的代码,一个关键点就是没有把不同的东西分解出来。如果我们用设计的眼光衡量这段代码,这就是“分离关注点”没有做好,把不同层面的东西混在了一起,既有不同业务混在一起,也有不同层次的处理混在了一起。
一次加一点
有时,一段代码一开始的时候并不长,但是慢慢的随着新的需求的增加或者bug的修复,我们会一点一点的增加代码,而一个有生命力的项目经常会延续很长的时间,于是,这段代码慢慢的变得越来越惨目忍睹了。
后来人再看到这段代码的时候就想骂人了。当他从版本控制的历史中找到这些代码的作者,去询问这些处理的来龙去脉时,每个人其实都很委屈,他们当时也没做太多,只是加了一个判断条件或者一段短的新逻辑而已。
任何代码都经不起这种无意识的累积,每个人都没做错,但最终的结果很糟糕。
对抗这种逐渐糟糕腐坏的代码,我们需要知道“童子军军规”:
让营地比你来时更干净。
—— 童子军军规
Robert Martin 把它借鉴到了编程领域,简言之,我们应该看看自己对于代码的改动是不是让原有的代码变得更糟糕了,如果是,那就改进它。