读书笔记 | 代码之丑

·  阅读 391

前言

本篇文章是阅读郑晔老师的<<代码之丑>>后的感悟和总结,收获颇多。

正文

"写代码"有俩个维度:正确性和可维护性,把代码写对,是每一个程序员的必备技能,能够把代码写的具有可维护性,这是一个程序员从业务迈向职业的第一步。(这句话另一位C++大佬也说过类似的:写给机器读得懂的代码不是本事,要能写出人能读得懂的代码。)

关于如何写出可维护性的代码,有很多经典书籍,比如<<程序设计实践 >>、<<代码整洁之道>>、<<重构>>等,但是都无法避免一个无情的事实:程序员们大多会认同这些书上的观点,但是每个人对于这些观点的理解却是千差万别的

比如书上说"命名是要有意义的",或许很多人只是觉得不用"abc"这种命名就是有意义,但是命名有意义远不止如此,比如代码中常用的"Info"、"Data"、"Manager"等都可能是没有意义的,都是反面代码。这些反面代码,在<<重构>>这本书中,起了一个名字,叫做代码的坏味道(Bad Smell)

对于代码中的坏味道,往往是很难发现的,因为它们不像代码Bug这么容易发现,所以郑老师就以代码中的坏味道来说,说如何发现代码中常见的坏味道,以及如何解决。

这里有一份总结,也叫做"坏味道自查表",后面内容会每项单独分析,总结如下:

  • 命名:
  1. 命名是否具有业务含义;
  2. 命名是否符合英语语法;
  • 函数:
  1. 代码行是否超过__行;
  2. 参数列表是否超过__个;
  1. 类的字段是否超过__个;
  2. 类之间的依赖关系是否符合架构规则;
  • 语句
  1. 是否使用for循环;
  2. 是否使用else;
  3. 是否有重复的switch;
  4. 一行代码中是否了连续的方法调用;
  5. 代码中是否出现了setter;
  6. 变量声明之后是否有立即再赋值;
  7. 集合声明之后是否有立即添加元素;
  8. 返回值是否可以使用Optional;

上面的每一个部分,后面会详细说明为什么是坏味道,以及如何来修改。

命名

有一句叫做"当你开始思考命名时,就说明你在进阶了",前面说了命名的最基本原则就是要有意义,那什么样子的命名是无意义的,是坏味道呢?下面列举几种坏味道的命名。

不精准的命名

看一段代码:

public void processChapter(long chapterId) {
  Chapter chapter = this.repository.findByChapterId(chapterId);
  if (chapter == null) {
    throw new IllegalArgumentException("Unknown chapter [" + chapterId + "]");  
  }
  
  chapter.setTranslationState(TranslationState.TRANSLATING);
  this.repository.save(chapter);
}
复制代码

这段代码看起来没有问题,通过阅读代码,我们知道该方法信息:方法名为processChapter即处理章节,传递进来一个章节Id,在方法内部先从repository中获取章节Chapter,然后修改其翻译状态为TRANSLATING(翻译中),最后再进行保存。

这个方法问题是出在方法名processChapter(处理章节)上,这个方法确实是在处理章节,但是这个名字太过宽泛,这里把"章节状态设置为翻译中"叫做处理章节,那么把"章节状态设置为翻译完"、"修改章节内容"等操作是不是都可以叫做处理章节,即如果很多场景都能够叫做处理章节,那么处理章节就是一个过于宽泛的名字,没有错,但是不精准

这是一种典型的命名坏味道,表明来看名字是有意义的,但是不能有效地反映这段代码的含义,必须花时间和精力去阅读其中的具体逻辑,这也是部分代码难以阅读的根源。

类似的不精准的命名我们常用的词有:data、info、flag、process、handle、build、maintain、manage、modify等,这些过于宽泛的名字很多时候都是因为在写代码的时候没有想好,就开始写代码了。

那如何修改呢?首先,命名要能描述这段代码在做的事情,比如前面代码做的事情是把"将章节改成翻译中",那方法名是否应该叫做changeChapterToTranslating呢?

不可否认,相比于processChapter,这个名字确实有进步,但是它还不算是一个好名字,因为它更多是在描述这段代码在做的细节。我们之所以要将一段代码封装起来,一个重要原因就是我们不想知道那么多细节,如果把细节平铺开来,那本质上和直接阅读代码细节差别并不大

所以,一个好的名字应该描述意图,而非细节

就这段代码而言,为什么要把翻译状态设置为翻译中,了解了业务后,这是因为在这里开启了一个翻译的过程,所以这段代码更应该命名为startTranslation:

public void startTranslation(long chapterId) {
  Chapter chapter = this.repository.findByChapterId(chapterId);
  if (chapter == null) {
    throw new IllegalArgumentException("Unknown chapter [" + chapterId + "]"); 
  }
  
  chapter.setTranslationState(TranslationState.TRANSLATING);
  this.repository.save(chapter);
}
复制代码

用技术术语命名

我们接着来看一段代码:

List<Book> bookList = service.getBooks();
复制代码

可以说这是一段常见得不能再常见的代码了,但是这段代码却隐藏了一个典型问题:用技术术语命名

这个bookList变量之所以叫做bookList,原因是它声明的类型是List,这种命名随处可见,比如xxxMap、xxxSet等。

这是一种不费脑子的命名方式,但是这种命名会带来很多问题,因为它是一种基于实现细节的命名方式

编程有一个重要的原则是面向接口编程,这个原则从另一个角度理解,就是不要面向实现编程,因为接口是稳定的,而实现是易变的。大多数人理解是,这个原则是针对类型的,但是在命名上,也应该遵循这个原则。

为什么呢?比如我发现现在需要的是一个不重复的作品集和,也就是说这个变量的类型从List改成Set,变量类型很容易改,但是所有的变量名都能确保都改好吗?假如漏了一个,就会出现一个叫做bookList的变量,它的类型是一个Set。

和前面一样,我们需要一个更面向意图的名字,比如这段代码我们就是想拿到一堆书,所以直接命名为books:

List<Book> books = service.getBooks();
复制代码

这是发现,这种更表意的名字,是一个更有效的名字。

还有一种,事实上,在实际的代码中,技术名称的出现,往往就代表着它缺少了一个应用的模型

比如,在业务代码中直接出现了Redis这种技术名词,就说明缺少一个中间层来充当模型,比如下面代码:

public Book getByIsbn(String isbn) {
  Book cachedBook = redisBookStore.get(isbn);
  ...
  return book;
}
复制代码

这是一段业务代码,但是出现了Redis,通常来说,业务需求是从缓存获取一个数据,而Redis则是一种实现而已,我们可以增加一个模型cache,如下:

public Book getByIsbn(String isbn) {
  Book cachedBook = cache.get(isbn);
  ...
  return book;
}
复制代码

这种情况下,至于具体缓存是如何实现的,在业务层我们不用关注。

程序员之所以喜欢用技术名称去命名,一个重要原因是在学习写代码时,很大程序参考了别人代码,而行业优秀的代码往往是一些开源项目,在一个技术类的项目中,这些技术术语就是它的业务语言,但是对于业务项目,这个说法就必须重新审视了

用业务语言写代码

不论是不精准的命名,还是技术名称命名,归根结底是一个问题:对业务理解不到位

想编写可维护的代码,必须要使用业务语言。从团队的角度看,让每个人根据自己的理解来命名,确实会出现千奇百怪的名字,所以一个良好的团队实践是建立团队的词汇表,让团队成员有信息可以参考。

对于业务语言理解导致的坏味道,就比较难以发现,比如下面方法声明:

public void approveChapter(long chapterId, long userId) {
  ...
}
复制代码

这个方法的意图是确定章节内容审核通过,这里有一个问题,chapterId是审核章节的ID,这个没问题,但是这个userId是什么呢?通过了解背景,我们知道,这个userId是审核人的userId,因为在审核时需要记录审核人信息。

通过业务分析,我们会发现这个userId并不是一个好的命名,因为需要更多的了解才知道这个命名的含义,所以这里更好的命名是审核人的Userid,即可以修改为reviewerUserId:

public void approveChapter(long chapterId, long reviewerUserId) {
  ...
}
复制代码

这种必须了解业务才能发现的坏味道,需要我们在写代码时,清晰地明白业务流程,这样不仅可以消除坏味道,还可以写出更容易维护地代码。

小节

坏味道:缺乏业务含义的命名。

错误命名:

  • 宽泛的命名。
  • 用技术术语命名。

命名遵循的原则:

  • 描述意图,而非细节。
  • 面向接口编程,接口是稳定的,实现是易变的。
  • 命名出现技术名称,往往是缺少一个模型。
  • 使用业务语言。

乱用英语

英语是程序员绕不开的一个槛,抛去本篇文章的主题,英语也是广大程序员必须要去学习的,因为只有真正去阅读英语文档或者一些API的英语注释,才能确保最高保真理解其含义,而不是直接查看通过翻译软件翻译出来的可能失真的文档。

继续本篇文章的主题,现在主流的语言都是以英语为基础的,所以想成为一个优秀的程序员,必须要会用英语。这里不要求程序员的英语有多好,但最低限度的要求是写出来的代码要像是在用英语表达

对于拼音和中文编程,现在的语言都是支持的,这些一眼就能看出的坏味道,本篇文章不做讨论,我们讨论几种不易发现的坏味道。

违反语法规则的命名

还是看一段代码:

public void completedTranslate(final List<ChapterId> chapterIds) {
  List<Chapter> chapters = repository.findByChapterIdIn(chapterIds);
  chapters.forEach(Chapter::completedTranslate);
  repository.saveAll(chapters); 
}
复制代码

这段代码看起来没啥问题,就是把一些章节的信息标记为翻译完成,但是仔细发现这里的方法名为completedTranslate,或许作者想表达"完成翻译"这个意图,所以完成还用了完成时的completed,翻译使用translate,但是这个名字还是起错了。

一般来说,常见的命名规则是:类名是一个名词,表示一个对象,而方法名则是一个动词,或者是动宾短语,表示一个动作

我们以这个为标准,completedTranslate就不是一个合格的动宾结构,这里我们只需要把完成改成complete,翻译改成名称形式即translation即可,所以修改后方法名为completeTranslation:

public void completeTranslation(final List<ChapterId> chapterIds) {
  List<Chapter> chapters = repository.findByChapterIdIn(chapterIds);
  chapters.forEach(Chapter::completeTranslation);
  repository.saveAll(chapters); 
}
复制代码

这不是一个难以察觉的坏味道,而且经常在我们代码里出现,这里需要上面的命名规则即可。

不准确的英语词汇

比如下面代码,定义了枚举,来标识章节的审核状态:

public enum ChapterAuditStatus {
    PENDING,
    APPROVED,
    REJECTED;
}
复制代码

这里的命名或许看不出问题,因为把"审核"这个单词放入翻译查询,确实会得到audit这个单词,但是同样是审核,在其他地方却有使用review这个单词,这就造成了不一致的问题。

造成这个问题的原因是,直接在翻译软件上,"审核"翻译为audit和review都有,而由于中英文的表达差异也没有太多人去理解。这里想统一,就需要真正结合业务去理解每个单词真正的偏重点,经过搜索可以发现audit有更官方的味道,类似翻译是审计,而review则更多是审查的意思,所以review更合适,上面代码就改成了:

public enum ChapterReviewStatus {
    PENDING,
    APPROVED,
    REJECTED;
}
复制代码

相比之下,这种坏味道就是一种高级的坏味道,英语单词用的不准确确实是中国程序员的一个短板。在这种情况下,最好的解决方案是建立起一个业务词汇表,千万不要臆想

建立业务词汇表的关键点就是用集体智慧,而非个体智慧,找到业务合适的通用语言,比如上面涉及章节审核的相关词汇,可以总结为词汇表:

名称英文说明
作品Book作者创造的作品
章节Chapter作品的一部分
审核review审核方对作品内容进行查看的过程
审核通过approve审核方对作品内容给予通过
审核未通过reject审核方对作品内容给予不通过

有了这种业务词汇表,以后小组内的程序员再也不用纠结命名了。

英语单词的拼写错误

一般来说,英语单词的拼写错误在IDE中都会有提醒,但是这里必须要注意一种特殊情况,就是把一个单词拼错误成了另一个单词。

比如下面代码:

public class QuerySort {
    private final SortBy sortBy;
    private final SortFiled sortFiled;
    ...
}
复制代码

这里的sortFiled本来是想表达"排序的字段"意思,但是把Filed拼写成了Filed,就容易让人理解为"排序文件"这种迷惑的意思,所以单词拼写错误一定要注意。

对于英语能力的提高,无他,只有强迫自己练习和总结,把常见的单词记忆,多看看优秀的开源项目,相比于完全掌握英语,掌握编程中涉及的英语还是要容易点。

小节

本小节没有讨论那种拼音命名、不恰当的命名(多个单词首字母)等这种低级的坏味道,而是讨论了几种不易发现的坏味道。

坏味道:乱用英语

英语使用不当:

  • 违反语法规则;
  • 不准确的英语词汇;
  • 英语单词拼写错误。

解决方法:

  • 制定代码规范;
  • 建立团队词汇表;
  • 经常性进行代码评审。

去除重复

重复是一个泥潭,对于程序员来说,要时刻提醒自己不要重复是至关重要的。在软件开发里,有一个重要的原则叫做 Don't Repeat Yourself(不要重复自己,简称DRY),更经典的叙述在<<程序员修炼之道>>中:在一个系统中,每一处知识都必须有单一、明确、权威地表述

所以本小节来看看常见的重复代码有哪些。

重复的结构

话不多说,还是来看几段代码:

@Task
public void sendBook() {
  try {
    this.service.sendBook();
  } catch (Throwable t) {
    this.notification.send(new SendFailure(t)));
    throw t;
  }
}
复制代码
@Task
public void sendChapter() {
  try {
    this.service.sendChapter();
  } catch (Throwable t) {
    this.notification.send(new SendFailure(t)));
    throw t;
  }
}
复制代码
@Task
public void startTranslation() {
  try {
    this.service.startTranslation();
  } catch (Throwable t) {
    this.notification.send(new SendFailure(t)));
    throw t;
  }
}
复制代码

这是3个业务函数,就是发送作品、发送章节和开始翻译,看起来这3个函数已经写的非常简洁了,但是这3段代码在结构上却有重复的,就是其中的catch语句。

先理解一下业务,这里进行catch的目的就是为了防止系统出了问题无人发掘,所以这里通过notification给飞书发送一个通知;相比于原来的逻辑,这个逻辑是后来加上的,所以代码作者不厌其烦的在每一处都添加了这行代码。

虽然这3个函数调用的业务代码不同,但是结构是一致的,有重复的结构,我们可以把其中重复的结构即捕获异常部分给提取出来:

private void executeTask(final Runnable runnable) {
  try {
    runnable.run();
  } catch (Throwable t) {
    this.notification.send(new SendFailure(t)));
    throw t;
  }
}
复制代码

有了这个结构,前面几个函数就可以用来改写了,对于支持函数式编程的程序语言来说,可以用语言提供的便利写法来简化代码,如下:

@Task
public void sendBook() {
  executeTask(this.service::sendBook);
}
复制代码
@Task
public void sendChapter() {
  executeTask(this.service::sendChapter);
}
复制代码
@Task
public void startTranslation() {
  executeTask(this.service::startTranslation);
}
复制代码

经过这个例子的改写,如果再有一些通用结构的调整,比如出错了需要加一些日志,就更好处理了。

这个例子非常简单,但是有2点需要注意:发现结构重复和思维转变

对于结构重复,一般是因为业务变更,或者程序员为了方便直接复制粘贴现有的方法代码,没有做过多考虑而导致的。

对于思维转变,我为什么要这么说呢?在上一节中,我们说命名时,类一般是名称,而函数是动词或者动宾结构,而对于参数一般是名词,这对于Java 8之前是完全没有问题的。

但是随着函数式编程的兴起,比如Kotlin和Java 8支持lambda,都是以函数为主角,即高阶函数可以作为参数。而函数是啥,函数是动作,这里的参数就不仅仅是名词,也可以是动词了。

比如上面的例子中,修改后的代码,就是传递动作这一典范,所以随着编程语言的进步,我们的编程思维也需要改变和提高。

做真正的选择

还是先来看一段代码:

if (user.isEditor()) {
  service.editChapter(chapterId, title, content, true);
} else {
  service.editChapter(chapterId, title, content, false);
}
复制代码

这里的逻辑是这样的:editChapter是用来编辑章节的,最后一个参数表示是否审核通过,而user可能是作者也可能是编辑,当是编辑时,自动审核通过,当是作者时则默认是审核不通过(因为无权审核)。

初看这段代码,感觉没啥毛病,这里也就是使用if来分开了2个不同的业务处理流程,但是仔细观察后,我们发现2个分支调用的函数仅仅是最后一个参数不一样。

这也是一种重复代码,造成这个原因的是作者在写这段代码时,脑子只想到if语句判断之后要做什么,而没有想到这个if语句判断的到底是什么。

写代码要有表达性,要能准确地把意图表达出来,是写代码过程中非常重要的一环。显然,这里的if判断是为了区分参数,而不是动作,所以我们可以稍微调整一下:

boolean approved = user.isEditor();
service.editChapter(chapterId, title, content, approved);
复制代码

注意这里的关注点,并没有直接把user.isEditor()当成参数传递给editChapter函数,原因是对于editChapter函数来说,最后一个Boolean类型的参数表示的是:是否审核通过,而不是这个用户是不是编辑。

这里单独用一个approved变量,就更容易读懂代码。

小节

发现和去除重复结构,在我们写代码中至关重要,而最难的就是挑战我们编程习惯,能够发现重复。发现重复,一种是我们已经在泥潭中挣扎了许久才后知后觉需要修改重复代码;一种是提升自己的识别能力,能主动发现重复。

而这种主动发现的能力,其实就需要对软件设计有更好的理解,尤其是分离关注点的理解,这也是本篇文章一直贯穿的主题。

坏味道:重复代码

重复的代码:

  • 复制粘贴的代码。
  • 结构重复的代码。
  • if和else代码块中的语句高度类似。

消灭重复代码原则:

  • 每一处知识都必须有单一、明确、权威地描述。

需要注意:

  • 转变思维,结构重复也是重复代码,要创新地看待高阶函数使用以及动作的重复。

长函数

说起长函数,这对于程序员来说就再也熟悉不过了,不论是复杂的业务代码,还是一些源代码,总有一些几百甚至上千行的长函数。

每当在长长的函数体中找到问题所在,再小心翼翼的改动代码,都是一些不愉悦的回忆。

这个点的问题,大家都知道,但是在说长函数之前,我们需要知道一个点:就是多长的函数才算长?

多长的函数才算长

为什么要讨论这个呢?不同的开发团队和不同的开发语言对于长函数的容忍度是不一样的,比如团队认为100行才算长函数,低于100行的不算,那绝大多数的函数都没有必要进行优化。

对于函数长度容忍度高,这是导致长函数产生的关键点

一个好的程序员面对代码库时要有不同尺度的观察能力,看设计时,要能够高屋建瓴,看代码时,要能细致入微

而对于长函数的长定义也是一样的,回到工作中,"越小越好"是一个追求的目标,只有有一个严格的标准,对代码长度容忍度降低,才会提供对细节的感知力,从而发现原本所谓细枝末节的地方隐藏的问题。

所以对于Java语言,这里建议长函数的定义是20行,当然不是一个强制标准,当然是越短越好,当一些业务无法拆分时,偶尔超过20行也是可以的。

长函数的产生

这里我们要知道长函数产生的一些原因,如果不理解长函数产生的原因,就很难在我们自己写代码时时刻提醒自己,下面列举几个长函数产生的原因。

以性能为由

我们都知道,函数调用的过程其实是一个入栈出栈的过程,当函数越多时,入栈出栈的次数就越多,这样会导致性能下降。

但是随着硬件的发展,和编译器、语言本身的优化,性能优化不应该是写代码的第一考量,更不应该拿这一点来写出长函数。

平铺直叙

在文章前面我们也说过,定义函数就是为了把一类动作封装,让人可以一眼看出函数意图,但是很多函数实现采用平铺直叙的方式,洋洋洒洒写了几百行,尤其对于复杂的流程,这样就容易产生长函数。

对于平铺直叙的代码风格,会有2个典型问题:

  1. 把多个业务处理流程放在一个函数里实现;
  2. 把不同层次的细节放到一个函数里实现。

这2个问题就会导致代码逻辑分层混乱,不符合单一职责原则。

而解决长函数的方法就是提取函数,把一个大函数拆分为若干个小函数。在拆分过程中,时刻铭记"分离关注点"这个原则,把不同的流程、不同的层次的代码给分离开来。

对于提取后的函数,还有一个特点,就是函数名更容易取名和理解。比如很多长函数,逻辑非常多,只能用handleXXX来表示,或者用一个非常长的函数名来表明其意图,这在上一节中我们说过,这是一种坏味道。当长函数不存在时,我们也更容易起名,更容易理解函数。

一次加一点

这个场景就更常见了,我们难免会维护一些旧代码,在旧代码上新增功能,为了最小改动,我们经常只在需要的地方加一点点代码,比如最开始的代码如下:

if (code == 400 || code == 401) {
  // 做一些错误处理
}
复制代码

随着后面需求越来越多,日积月累,每一次就改一点,可能会变成:

if (code == 400 || code == 401 || code == 402 || ...
  || code == 500 || ...
  || ...
  || code == 10000 || ...) {
  // 做一些错误处理
}
复制代码

这时再去阅读,就发现很难理解了。

任何代码都经不起这种无意识的积累,每个人都没有错,但是结果很糟糕

对于这种问题该如何做呢?总不能不增加需求吧,这里有一个"童子军军规":让营地比你来时更干净

在编程领域也是这样,如果我们自己对代码的改动让原有代码变得更糟糕,就需要去改进它。而这一切的前提是,自己要能发现是否会变得糟糕,所以识别代码坏味道非常必要。

小节

坏味道:长函数。

长函数的产生:

  • 以性能为由;
  • 以平铺直叙方式写代码;
  • 一次增加一点代码;

消灭长函数的原则:

  • 定义好长函数的标准。
  • 做好"分离关注点",拆分长函数。
  • 改代码时,坚守"童子军军规"。

总结

本篇读书笔记就先到这里,最后一点感悟,就是不论是写什么代码,都时刻要记住把代码写好,要有工匠精神,编程是一门终身学习和专研的技术。

最后强烈建议阅读源文章和几本文章中提及的书籍,感兴趣的可以阅读源文章:time.geekbang.org/column/intr…

分类:
Android
标签:
收藏成功!
已添加到「」, 点击更改