

代码之丑

郑晔

推文科技技术 VP，前火币网首席架构师

2194 人已学习

立即订阅

登录后，你可以任选2讲全文学习

课程目录

已完结/共 22 讲

开篇词 (2讲)



开篇词 | 这一次，我们从“丑”代码出发

课前热身 | 这些需求给到你，你会怎么写代码？

13类典型坏味道 (13讲)



01 | 缺乏业务含义的命名：如何精准命名？

02 | 乱用英语：站在中国人的视角来看英文命名

03 | 重复代码：简单需求到处修改，怎么办？

04 | 长函数：为什么你总是不可避免地写出长函数？

05 | 大类：如何避免写出难以理解的大类？

06 | 长参数列表：如何处理不同类型的长参数？

07 | 滥用控制语句：出现控制结构，多半是错误的提示

08 | 缺乏封装：如何应对火车代码和基本类型偏执问题？

09 | 可变的数据：不要让你的代码“失控”

10 | 变量声明与赋值分离：普通的变量声明，怎么也有坏味道？

11 | 依赖混乱：你可能还没发现问题，代码就已经无法挽救了

12 | 不一致的代码：为什么你的代码总被吐槽难懂？

13 | 落后的代码风格：使用“新”的语言特性和程序库升级你的代码

07 | 滥用控制语句：出现控制结构，多半是错误的提示

郑晔 2021-01-14

你好，我是郑晔。
在前面几讲，我们已经讲了不少的坏味道，比如长函数、大类等。对于有一定从业经验的程序员来说，即便不能对这些坏味道有一个很清楚的个人认知，但至少一说出来，通常都知道是怎么回事。
但这节课我要讲的坏味道对于很多人来说，可能就有点挑战了。这并不是说内容有多难，相反，大部分人对这些内容简直太熟悉了。所以，当我把它们以坏味道的方式呈现出来时，这会极大地挑战很多人的认知。
这个坏味道就是滥用控制语句，也就是你熟悉的 if、for 等等，这个坏味道非常典型，但很多人每天都用它们，却对问题毫无感知。今天我们就先从一个你容易接受的坏味道开始，说一说使用控制语句时，问题到底出在哪。
嵌套的代码我给你看一张让我印象极其深刻的图，看了之后你就知道我要讲的这个坏味道是什么了。
图片来源于网络
相信不少同学在网上见过这张图，是的，我们接下来就来讨论嵌套的代码。
考虑到篇幅，我就不用这么震撼的代码做案例了，我们还是从规模小一点的代码开始讨论：
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);
      }
    }                                            
  }
}
这是一段做 EPUB 分发的代码，EPUB 是一种电子书格式。在这里，我们根据作品 ID 找到要分发的 EPUB，然后检查 EPUB 的有效性。对于有效的 EPUB，我们要为它注册 ISBN 信息，注册成功之后，将这个 EPUB 发送出去。

公开

同步至部落

取消

完成

0/1000字

划线

笔记

复制

开通超级会员免费畅看本课程

开通会员

该文章仅可免费阅读部分内容，如需阅读完整文章，请开通超级会员或单独购买本课程。

登录后留言

精选留言(28)

于途
以卫语句取代嵌套的条件表达式。我在第一家公司转正后，组内code review ，我们组长就推荐了这种做法，一直沿用到现在😁，只是不知道“卫语句”这个正式的概念！
作者回复: 学习一些行业通用的语言还是需要的。
2021-01-14
3
14
呆呆狗的兽
你好，我看您建立了很多与枚举项一一对应的内部类，这些内部类的创建在那个位置与枚举对象本身绑定？还是说枚举UserLevel(枚举) implements UserLevel(接口)？

我一般都是习惯将这些逻辑封装在枚举的实现里具体代码我一般会实现为：

public enum UserLevel {
    SILVER() {
        @Override
        public double getPrice(Book book) {
            return book.getPrice() * 0.9;
        }

        @Override
        public double getPrice(Epub epub) {
            return epub.getPrice() * 0.85;
        }
    },
    GOLD() {
        @Override
        public double getPrice(Book book) {
            return book.getPrice() * 0.8;
        }

        @Override
        public double getPrice(Epub epub) {
            return epub.getPrice() * 0.85;
        }
    },
    PLATINUMP() {
        @Override
        public double getPrice(Book book) {
            return book.getPrice() * 0.75;
        }

        @Override
        public double getPrice(Epub epub) {
            return epub.getPrice() * 0.8;
        }
    };

    public abstract double getPrice(Book book);

    public abstract double getPrice(Epub epub);
}

然后调用的地方：

public double getBookPrice(final User user, final Book book) {
    UserLevel level = user.getUserLevel();
    return level.getPrice(book);
}

public double getBookPrice(final User user, final Epub epub) {
    UserLevel level = user.getUserLevel();
    return level.getPrice(epub);
}

请问这里我用此种方式实现，是否有什么不妥？
我个人在项目中几乎很偏向于用枚举，来封装很多业务的不同性（业务针对不同枚举的实现与判断，都放在了枚举的方法实现中）
是否有问题？希望得到郑老师解答，感谢

作者回复: 没什么不妥，实际上，我也经常这么做。在这个例子里，我选择了大家更容易理解的方式，适用面更广一些而已。
2021-02-07

11
Demon.Lee
郑大，请教两个问题，谢谢。
“对象健身操”中有这样几句话：
1）规则2：拒绝else关键字
“需要小心的是，如果过度使用“提前返回”，代码的清晰度很快会降低。”，
“面向对象编程语言给我们提供了一种更为强大的工具——多态。它能够处理更为复杂的条件判断。对于简单的条件判断，我们可以使用“卫语句”和“提前返回”替换它。”
这里的“卫语句” 和 “提前返回” 是一个意思么，我理解他们是一样的，都是提前check并return。

2）规则4：一行代码只有一个“.”运算符
“迪米特法则（The Law of Demeter，“只和身边的朋友交流”）是一个很好的起点。还可以这样思考它：你可以玩自己的玩具，可以玩你制造的玩具，还有别人送给你的玩具。但是永远不要碰你的玩具。”
什么叫 “不要碰你的玩具”？是不是 “不要碰别人的玩具”？
作者回复: 卫语句是前提条件，结果是提前返回。

至于你的第二个问题，看不懂中文时，就去看英文。

The Law of Demeter (“talk only to your friends”) is a good place to start, but think about it this way: you can play with your toys, with toys that you make, and with toys that someone gives you. You don’t ever, ever play with your toy’s toys.

人家的原文是 You don’t ever, ever play with your toy’s toys. 别动玩具的玩具，和迪米特法则说的是一回事，所以，你不理解的原因是翻译的不好。
2021-01-16
3
7
业余爱好者
工作两年，从未用过switch
作者回复: 好样的！
2021-01-14

5
黑皮诺
郑老师，

以UserLevel为例，假设我需要提供一个api, 用户端提供request body，我（service端）需要把request body serialize成一个UserLevel （body会额外提供一个usertype是enum(regular, gold, silver)，之后需要getBookPrice()。

问题是，serialize之后没办法把UserLevel cast成具体的RegularUserLevel/GoldUserlevel，至少c#不允许把父类cast成子类。我现在的解决方案是写了一个parser, 根据usertype写了一个switch语句，每个子类的构造函数接受一个父类 GoldUserLevel(UserLevel ul)，然后把成员完全copy。感觉两个坏味道正在产生, 想请教一下，在这种情况下您会怎么处理呢？

1. 是否不同的 userlevel 需要提供不同的api?
2. 假设需求就是提供一个api解决所有的userlevel，这种情况下的best practice是怎样呢？

谢谢！
作者回复: 这是一个好问题。

我不建议针对不同的用户级别提供不同的 API，因为这是和具体业务演进强相关的，每添加一个用户级别，API 都要修改，稳定性非常差。

这里一定要有防腐层的概念在心里，API 接口是外部的，它里面传输的内容不一定和业务是一一映射的。所以，把传输中的用户级别通过一个工厂（factory）转换成一个业务对象是非常正常的。在第 11 讲会有讲到一个类似的问题。

这里的重点就是提供 API，就要思考 API 应该怎么设计，然后是，API 和业务对象之间如何映射。
2021-01-14

4
Hobo
真的棒，我现在写代码也是尽量只用if避免else，可读性比原来ifelse好太多
作者回复: 肉眼可见的进步。
2021-01-14

3
Geek_3b1096
一直以来认为if-else成对出现
作者回复: 就是要打破这种认知。
2021-01-18

2
qinsi
smalltalk里没有控制结构；lisp里没有循环
作者回复: 同样的事，可以有不同的做法。
2021-01-15

2
adang
对于if...else这种情况，印象很深刻，刚刚入行写代码的时候，TeamLeader就讲，不要写很长的if.....else，这样的代码，看了半天到最后发现还有一个else情况要处理，代码读进来太费劲，要把异常情况先处理掉先返回，这样代码看起来比较清爽。后面写代码的时候，也会按照这种思路来处理。在其他团队看到的代码，绝大多数情况下都是if...else if..else 这样平铺着写，常常怀疑自己的写法是不是错的:(。对于重复switch这种情况，真不知道是有这样的优化方案的，好好收藏。
作者回复: 进一步有一步的欢喜。
2021-01-14

2
AFlymamba
现象：检查原有自己代码：发现曾经有过这段else的坏味道
JSONObject ticketInfo = this.getTicket(corpid, agentid);
if (null == agentid) {
    ticketInfo = this.getJsApiTicket(corpid);
} else {
    ticketInfo = this.getAgentJsApiTicket(corpid, agentid);
}
改进：用了两种方式去调整：
方式一：提取方法，内部用三目表达式
private JSONObject getTicket(final String corpid, final Long agentid) {
  return null == agentid ? this.getJsApiTicket(corpid) : this.getAgentJsApiTicket(corpid, agentid);
}
方式二：采用return，卫语句去提前返回
private JSONObject getTicketMethodTwo(final String corpid, final Long agentid) {
      if (null == agentid) {
            return this.getJsApiTicket(corpid);
      }
      return this.getAgentJsApiTicket(corpid, agentid);
}

最终，回到调用点：
JSONObject ticketInfo = this.getTicket(corpid, agentid);
总结：多思考，不同角度用不同标准去自问，小步重构和优化，小步提交。
作者回复: 多谢分享！
2021-02-02

1
王啸
循环也是坏味道，是我万万没想到的
作者回复: 要的就是“万万没想到”
2021-01-26

1
return
老师讲的很好，感谢老师，
再请教一下老师，文中的这行代码
UserLevel level = user.getUserLevel()
getUserLevel 这个方法如何写才是好的，归根到底是不是还得判断。
作者回复: switch本身不是问题，问题是重复。
2021-01-16
2
1
Abcd
switch也不是毒药，比如用在"循环展开"，"达夫设备"上还是很不错的
作者回复: 优化的场景要单独算，那都是非常规写法。
2021-01-15
2
1
Tio Kang
有一个问题，else语句以CC衡量是不影响代码复杂度的。那么是否实际上不写else的原因如下：1.减少代码行数；2. 避免else带来的潜在的嵌套可能
作者回复: 主要是为了减少嵌套。
2021-01-14

1
刘大明
代码坏味道之滥用控制语句,这个在项目中也非常常见。可惜评论不能发图片。不然就发上来看看。
这段时间一直在纠结要不要离职，远离这些烂代码。但是怎么学习重构烂代码不也是一种更好的挑战吗？
我想如果能把公司的烂代码重构让代码更好看，那也是一种能力的提升。
作者回复: 可以发到部落里的代码吐槽大会，让大家开开眼。
2021-01-14

1
爱吃彩虹糖的猫~
看到这篇，我突然醒悟，郑是想要将《重构》中的内容揉碎了，取合适的讲解出来吗？😊
作者回复: 嗯，可以这么理解，也讲一些《重构》没讲的。
2021-01-14

1
沉默着走，用心感受/shx
switch语句用多态重构，看了n遍表示没看懂
2022-01-08


Peter
郑大：
public double getBookPrice(final User user, final Book book) {
  UserLevel level = user.getUserLevel()
  return level.getBookPrice(book);
}

public double getEpubPrice(final User user, final Epub epub) {
  UserLevel level = user.getUserLevel()
  return level.getEpubPrice(epub);
}

想问下你这么重构完之后，是不是UserLevel level = user.getUserLevel() 这个函数里面会有switch case的存在呢？是不是我可以理解这种重构只是消除了switch case的重复代码并不能消除switch case这种呢？

2021-12-29


Peter
想请教下如果你上面那部分重构代码处理的话，我把for循环放到第二个函数里面的话，会有啥不好的地方吗
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);
    }
  }
}
2021-12-29


刘羽禅
只有一层的 if else ,不优化也行的.
只要团队成员一眼都能看懂就没问题, 有问题的是嵌套太深的if else,
我认为,完全不写else 也是一种过犹不及的想法.
2021-12-28



收起评论

28









下载
客户端

返回
顶部