原文链接：www.yuque.com/xiazhiqiang… 《研发流程--代码质量》
写代码的两个维度
- 正确性 ：代码写对，是程序员的必备技能
- 可维护性：但能够把代码写得更具可维护性，这是一个程序员从中级迈向高级的第一步
代码整洁之道
- 《重构》，《代码整洁之道》
测试
- 敬畏代码，任何的改动一定要测试
典型代码坏味道
不精准的命名
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);
}
// changeChapterToTranslating
1、命名要能描述代码正在做的事情
2、命令应该描述意图，而不是细节
- 用技术术语命名
List<Book> bookList = service.getBooks();
// 可以说这是一段常见得不能再常见的代码了，但这段代码却隐藏另外一个典型得不能再典型的问题：用技术术语命名。
// 编程有一个重要的原则是面向接口编程，这个原则从另外一个角度理解，就是不要面向实现编程，因为接口是稳定的，而实现是易变的
// 那有什么更好的名字吗？我们需要一个更面向意图的名字，其实，我们在这段代码里真正要表达的是拿到了一堆书，所以，这个名字可以命名成 books。
List<Book> books = service.getBooks();
1、无论是不精准的命名也好，技术名词也罢，归根结底，体现的是同一个问题，对业务理解不到位。
2、命名是软件开发中难事之一，不好的命名本质上是增加我们的认知成本，同样也增加了后来人维护代码的成本。
3、好的命名要体现出这段代码在做的事情，而无需展开代码了解其中的细节，这是最低的要求，进一步，好的命名要准确地体现意图，而不是实现细节。更高的要求是，用业务语言写代码
乱用英语
public void completedTranslate(final List<ChapterId> chapterIds) {
List<Chapter> chapters = repository.findByChapterIdIn(chapterIds);
chapters.forEach(Chapter::completedTranslate);
repository.saveAll(chapters);
}
1、常见的命名规则是：类名是一个名词，表示一个对象，而方法名则是一个动词，或者是动宾短语，表示一个动作
// 以此为标准衡量这个名字，completedTranslate 并不是一个有效的动宾结构。 只要把“完成”译为 complete，“翻译”用成它的名词形式 translation 就可以了。所以，这个函数名可以改成 completeTranslation
不准确的英语词汇
public enum ChapterAuditStatus {
PENDING,
APPROVED,
REJECTED;
}
public enum BookReviewStatus {
PENDING,
APPROVED,
REJECTED;
}
1、这种情况下，最好的解决方案还是建立起一个业务词汇表, 包含业务术语的中英文表达。这样在写代码的时候，就可以参考这个词汇表给变量和函数命名
2、建立词汇表的另一个关键点就是，用集体智慧，而非个体智慧。你一个人的英语可能没那么好，但一群人总会找出一个合适的说法，业务词汇表也应该是构建通用语言的一部分成果
- 拼写错误
IntelliJ IDEA 这样的 IDE 甚至可以给你提示代码里有拼写错误
重复代码
1、重复代码
2、重复结构
建议，不要使用复制粘贴，真正应该做的是，先提取出函数，然后，在需要的地方调用这个函数
长函数
产生： 长函数本身是一个结果
问题: 把多个业务处理流程放在一个函数里实现；
把不同层面的细节放到一个函数里实现。
方式： 最简单的提取，提取之后命令更短(理解的成本也相应地会降低)
随着对代码长度容忍度的降低，对代码细节的感知力就会逐渐提升，你才能看到那些原本所谓细枝末节的地方隐藏的各种问题
一个好的程序员面对代码库时要有不同尺度的观察能力，看设计时，要能够高屋建瓴，看代码时，要能细致入微
大类
一个人理解的东西是有限的，没有人能同时面对所有细节
- 人类面对复杂事物给出的解决方案是分而治之
- 通常来说，很多类之所以巨大，大部分原因都是违反了单一职责原则
而想要破解“大类”的谜题，关键就是能够把不同的职责拆分开来
- 应对大类的解决方案，主要是将大类拆分成小类，我们需要认识到，模块拆分，本质上是帮助人们降低理解成本的一种方式
长参数列表
典型的消除长参数列表的重构手法：将参数列表封装成对象
滥用控制语句
public void distributeEpubs(final long bookId) {
List<Epub> epubs = this.getEpubsByBookId(bookId);
for (Epub epub : epubs) {
if (epub.isValid()) {
boolean registered = this.registerIsbn(epub);
if (registered) {
this.sendEpub(epub);
}
}
}
}
// 首先这段代码的，从实现功能的角度来说，这段代码肯定没错,按照需求一步一步地把代码实现出来了
// 但问题是，在把功能实现之后，而没有把代码重新整理一下
public void distributeEpubs(final long bookId) {
List<Epub> epubs = this.getEpubsByBookId(bookId);
for (Epub epub : epubs) {
this.distributeEpub(epub)；
}
}
private void distributeEpub(final Epub epub) {
if (epub.isValid()) {
boolean registered = this.registerIsbn(epub);
if (registered) {
this.sendEpub(epub);
}
}
}
通常来说，if 语句造成的缩进，很多时候都是在检查某个先决条件，只有条件通过时，才继续执行后续的代码。
这样的代码可以使用卫语句（guard clause）来解决，也就是设置单独的检查条件，不满足这个检查条件时，立刻从函数中返回。
private void distributeEpub(final Epub epub) {
if (!epub.isValid()) {
return;
}
boolean registered = this.registerIsbn(epub);
if (!registered) {
return;
}
this.sendEpub(epub);
}
public double getEpubPrice(final boolean highQuality, final int chapterSequence) {
double price = 0;
if (highQuality && chapterSequence > START_CHARGING_SEQUENCE) {
price = 4.99;
} else if (sequenceNumber > START_CHARGING_SEQUENCE
&& sequenceNumber <= FURTHER_CHARGING_SEQUENCE) {
price = 1.99;
} else if (sequenceNumber > FURTHER_CHARGING_SEQUENCE) {
price = 2.99;
} else {
price = 0.99;
}
return price;
}
//
public double getEpubPrice(final boolean highQuality, final int chapterSequence) {
if (highQuality && chapterSequence > START_CHARGING_SEQUENCE) {
return 4.99;
}
if (sequenceNumber > START_CHARGING_SEQUENCE
&& sequenceNumber <= FURTHER_CHARGING_SEQUENCE) {
return 1.99;
}
if (sequenceNumber > FURTHER_CHARGING_SEQUENCE) {
return 2.99;
}
return 0.99;
}
当代码里只有一层缩进时，代码的复杂度就大大降低了，理解成本和出现问题之后定位的成本也随之大幅度降低
缺乏封装
过长的消息链
String name = book.getAuthor().getName();
解决这种代码的重构手法叫隐藏委托关系（Hide Delegate），说得更直白一些就是，把这种调用封装起来：
String name = book.getAuthorName();
基本类型偏执
编程规则：1、尊重迪米特法则
2、封装基本类型和字符
可变的数据
setter
可变的数据会带来许多问题，你不知道数据会在哪里被何人以什么方式修改，造成的结果是，别人的修改会让你的代码崩溃。与之相伴的还有各种衍生出来的问题，最常见的就是我们常说的并发问题。
全局数据
1、限制变化
2、不可变的类
3、限制可变的数据
变量声明与赋值分离
EpubStatus status = null;
CreateEpubResponse response = createEpub(request);
if (response.getCode() == 201) {
status = EpubStatus.CREATED;
} else {
status = EpubStatus.TO_CREATE;
}
// 建议修改
private static Map<Locale, String> CODE_MAPPING = new HashMap<>();
...
static {
CODE_MAPPING.put(LOCALE.ENGLISH, "EN");
CODE_MAPPING.put(LOCALE.CHINESE, "CH");
}
问题 ： 这种代码真正的问题就是不清晰，变量初始化与业务处理混在在一起，当代码混在一起的时候，我们必须小心翼翼地从一堆业务逻辑里抽丝剥茧，才能把逻辑理清，知道变量到底是怎么初始化的。
建议 ： 1、变量初始化最好一次完成
2、 在能够使用 final 的地方尽量使用 final
依赖关系
关于防腐层的关系，还有待理解
不一致的代码
大多数程序员都是在一个团队中工作，对于一个团队而言，一致性是非常重要的一件事。因为不一致会造成认知上的负担，在一个系统中，做类似的事情，却有不同的做法，或者起到类似作用的事物，却有不同的名字，这会让人产生困惑。
public String nowTimestamp() {
DateFormat format = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
Date now = new Date();
return format.format(now);
}
public String nowTimestamp() {
LocalDateTime now = LocalDateTime.now()
return now.format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"));
}
主要是因为一个项目中，应对同一个问题出现了多个解决方案，如果没有一个统一的约定，项目成员会根据自己写代码时的感觉随机选择一个方案，这样的结果就是出现方案上的不一致。
public void createBook(final List<BookId> bookIds) throws IOException {
List<Book> books = bookService.getApprovedBook(bookIds)
CreateBookParameter parameter = toCreateBookParameter(books)
HttpPost post = createBookHttpRequest(parameter)
httpClient.execute(post)
}
// 首先是获取审核通过的作品，这是一个业务动作，接下来的三行其实是在做一件事，也就是发送创建作品的请求。具体到代码上，这三行代码分别是创建请求的参数，
// 根据参数创建请求，最后，再把请求发送出去。这三行代码合起来完成了一个发送创建作品请求这么一件事，而这件事才是一个完整的业务动作。
public void createBook(final List<BookId> bookIds) throws IOException {
List<Book> books = bookService.getApprovedBook(bookIds)
createRemoteBook(books)
}
private void createRemoteBook(List<Book> books) throws IOException {
CreateBookParameter parameter = toCreateBookParameter(books)
HttpPost post = createBookHttpRequest(parameter)
httpClient.execute(post)
}
// 原来的函数（createBook）里面全都是业务动作，而提取出来的函数（createRemoteBook）则都是业务动作的细节，各自的语句都是在一个层次上了。
能够分清楚代码处于不同的层次，基本功还是分离关注点
命名不一致
建议：1、团队统一编码方案
2、提取函数，将不同层次的内容放在不同的函数中，保持代码在各个层面上的一致性
落后的代码风格
Author author = book.getAuthor();
String name = (author == null) ? null : author.getName();
class Book {
public Optional<Author> getAuthor() {
return Optioanl.ofNullable(this.author);
}
...
}
Optional<Author> author = book.getAuthor();
String name = author.isPresent() ? author.get().getName() : null;
// 继续改进
Optional<Author> author = book.getAuthor();
String name = author.map(Author::getName).orElse(null);
函数式编程
public ChapterParameters toParameters(final List<Chapter> chapters) {
List<ChapterParameter> parameters = new ArrayList<>();
for (Chapter chapter : chapters) {
if (chapter.isApproved()) {
parameters.add(toChapterParameter(chapter));
}
}
return new ChapterParameters(parameters);
}
// 改进
public ChapterParameters toParameters(final List<Chapter> chapters) {
List<ChapterParameter> parameters = chapters.stream()
.filter(Chapter::isApproved)
.map(this::toChapterParameter)
.collect(Collectors.toList());
return new ChapterParameters(parameters);
}
// 循环语句是在描述实现细节，而列表转换的写法是在描述做什么，二者的抽象层次不同
建议：1、声明书编程
2、写职责单一，短小的函数
3、不断学习"新"代码分格
代码评审
为什么要做代码评审
-
- 没有人能够保证自己写出来的代码是没有问题的，而规避个体问题的主要方式就是使用集体智慧，也就是团队的力量
- 代码评审的过程，也是一个知识分享的过程，保证一些细节的知识不再是隐藏在某一个人的头脑中，而是放置到了团队的层面
及时评审
-
- 评审周期过长是有问题的，周期过长，累积的问题就会增多，造成的结果就是太多问题让人产生无力感
建议 ： 1、尽可能更多的暴露问题
2、尽可能的多做代码评审
评审的角度
1、实现方案的正确性
2、算法的正确性
3、代码的坏味道****
新需求
谨慎地对待接口和实体的变动。
建议
1、对我提供的接口越少越好
2、仔细分析实体扮演的角色
重构
重构就是一个模式匹配的过程，识别出坏味道，运用对应的重构手法解决问题
坏味道是一切重构的起点，而识别坏味道不是靠个人审美，而要依赖通用的标准
写代码是一门手艺，需要不断地打磨。
一方面，坚持写代码，保持自己对于代码的体感
另一方面，保持对于代码的敏感度，不断思考对于代码的改进，寻找更好的写法
