《代码整洁之道 Clean Code》学习笔记 Part 2 - 如何写出优雅的函数

发布时间 2023-09-03 22:14:10作者: Zijian/TENG

大师级程序员把系统当作故事来讲,而不是当作程序来写。

TLDR

  1. 短小(不超过 20 行、缩进不超过 2 层)
  2. 只做一件事
  3. 保持在同一抽象层级
  4. 用多态替代 switch
  5. 取个好的函数名
  6. 函数参数越少越好(尽量避免 3 个及以上参数)
  7. 无副作用、避免输出参数
  8. 分隔指令与询问
  9. 用异常替代错误码
  10. 不要重复(DRY: Don't Repeat Yourself)

1. 短小

这是函数的第一原则!作者几十年的的经验告诉我们:函数就应该短小,不接受反驳。

  • 最好不要超过 20 行
  • 缩进的层级不超过 2 层
  • if/else/while 里面的代码应该只有一行(通常是一句函数调用):这样不但能保持函数短小,因为函数名本身具有说明性的名称,从而增加了文档上的价值

2. 只做一件事

参见:单一职责原则 SRP

函数的目的是把一个大的概念拆分成另一个抽象层级上的一系列步骤。

要判断函数是否只做了一件事,就看能否再拆出“改变抽象层级”的函数。

3. 每个函数一个抽象层级

什么是抽象层级?举个例子:

  • genHtml() 位于较高抽象层
  • pagePathName = PathParser.render(pagePath) 位于中间抽象层
  • .append("\n") 位于较低的抽象层

如果一个函数中同时有这些不同层级的语句,那就违反了这个规则。

最佳实践:自顶向下读代码,每个函数后面跟着下一个抽象层级的函数。

4. 用多态替代 switch

switch 天生就要做 N 件事,很难写出短小的 switch 语句:

Money calculatePay(const Employee &e) {
  switch (e.type) {
  case COMMISIONED:
    return calculateCommisionedPay(e);
  case HOURLY:
    return calculateHourlyPay(e);
  case SALARIED:
    return calculateSalariedPay(e);
  default:
    throw InvalidEmployeeType(e.type);
  }
}

存在的问题:

  • 违反了 SRP 单一职责原则:有好几个修改它的理由
  • 违反了 OCP 开放关闭原则:每次添加新的员工类型都要修改这段代码

不仅如此,上述问题可能在多个地方重复出现:

bool isPayday(const Employee& e);
void deliverPay(const Employee& e, Money pay);
...

解决办法就是利用多态抽象工厂,将 switch 封装到工厂内部,让 switch 只出现一次,用于创建多态对象:

// 抽象基类 Employee
class Employee {
public:
  virtual bool isPayday() = 0;
  virtual Money calculatePay() = 0;
  virtual void deliverPay(Money pay) = 0;
};
// 用工厂封装 switch(只出现一次)
Employee *makeEmployee(...) {
  switch (...) {
  case COMMISIONED:
    return CommissionedEmployee(...);
  case HOURLY:
    return HourlyEmployee(...);
  case SALARIED:
    return SalariedEmployee(...);
  default:
    throw InvalidEmployeeType(type);
  }
}

isPayday、calculatePay、deliverPay 方法在具体子类中实现,对于调用 Employee 的代码来说,无需关心 Employee 的具体类型(面向接口编程)。和原来的实现相比:

  • 增加新的员工类型时,需新增一个 Employee 的子类,实现 Employee 的抽象方法。此外,唯一需要修改的地方就是工厂中的 switch 语句,而其他使用 Employee 的代码无需改动
  • 函数参数减少一个:不再需要传入 Employee

5. 使用描述性名称

  • 函数越短小,功能越集中,越容易取个好名字。
  • 长而清晰的名字好过短而费解(以至于不得不额外加注释)的名字
  • 追求好名字的过程有助于厘清设计思路,对代码进行改进重构
  • 命名风格保持一致

6. 函数参数

函数参数越少越好,尽量避免 3 个及以上参数:

  • 参数越多,代码越难以理解
  • 多个参数的函数难以测试:因为很难覆盖所有的排列组合
  • 避免使用 out 参数,应该直接返回输出内容(错误信息可以通过异常抛出或者返回 Result<T, Error> / std::optional<T>
  • 避免 bool 参数,如果存在 bool 类型的参数,大概率是做了两件事,应该拆成两个函数

减少参数的方法:

  • 把函数作为其中一个参数的成员:foo(a, b) --> a.foo(b)
  • 把其中几个参数封装为类:makeCircle(double x, double y, double r) --> makeCircle(Point center, double r)

7. 无副作用

副作用会导致奇怪的时序性耦合及顺序依赖:

bool checkPassword(string username, string password) {
  auto user = getUser(username);
  string codedPhrase = user.getPhraseEncodedByPassword();
  if(cryptographer.decrypt(codedPhrase, password) == "valid password") {
    initializeSession(); // 副作用
    return ture;
  }
  return false;
}

问题出在 initializeSession() 的调用:如果调用者只想核验密码而调用 checkPassword,就会意外重置当前 session。另外,这一副作用也导致了时序性耦合,即 checkPassword 只能在特定时候调用,在其他时刻调用则可能导致 session 数据丢失。如果一定要时序性耦合,至少应该在函数名中体现,如 checkPasswordAndInitializeSession,但这样还是违反了“只做一件事”的原则。

输出参数是另一种副作用。面向对象几乎不需要输出参数,如果函数需要修改某个状态,应该修改所属对象的状态。

8.分隔指令与询问

函数要么做什么事,要么回答什么事,不可以同时做两件事。应该避免这样的函数:

// 成功返回 true,属性不存在返回 false
bool set(string attr, string value);

这会导致这样的语句:

if(set("username","zijian"))
{
  // 如果不看注释,很难知道 set 函数到底做了什么,返回值代表什么含义
}

即使将 set 修改为一个更贴切的名字 setAndCheckIfExists 也不会对可读性有多少提升。真正的解决方案是将指令与询问分隔开来:

if(attributeExists("username")) {
  setAttribute("username", "zijian");
  ...
}

9. 使用异常替代错误码

指令式的函数返回错误码也违反了上一条“指令与询问分隔”的规则。

更讨厌的是,错误码会“强迫”调用者立即检查返回值,进而导致深层嵌套,难以阅读。你一定写过或见过类似的代码:

if (DeletePage(page) == E_OK) {
  if (registry.DeleteReference(page.name) == E_OK) {
    if (configKeys.DeleteKey(page.name.MakeKey() == E_OK)) {
      logger.Log("page deleted");
    } else {
      logger.Log("configKey not deleted");
    }
  } else {
    logger.Log("deleteReference from registry file failed");
  }
} else {
  logger.Log("delete failed");
  return E_ERROR;
}

而如果使用异常替代错误码,就可以将错误处理和主要逻辑分离开

try {
  DeletePage(page);
  registry.DeleteReference(page.name);
  configKeys.DeleteKey(page.name.MakeKey();
} catch (const std::exception &e) {
  logger.Log(e.what());
}

更进一步,try/catch 也应该和代码的正常流程分离:

void Delete(Page *page) {
  try {
    DeletePageAndAllReference(page);
  } catch (const std::exception &e) {
    LogError(e);
  }
}

void DeletePageAndAllReference(Page *page) {
  DeletePage(page);
  registry.DeleteReference(page.name);
  configKeys.DeleteKey(page.name.makeKey();
}

void LogError(const std::exception &e) {
  logger.Log(e.what());
}

一个函数只做一件事,而错误处理本身就是一件事。换句话说,如果一个函数中有 try/catch,那么这个函数的第一个单词就应该是 try,catch 之后不应该再有其他代码,每个 try/catch 语句中,应该只有一个语句/函数调用。

这么做的好处:

  • Delete() 函数只负责错误处理,阅读代码的时候大脑会“自动跳过”
  • 主要逻辑函数 DeletePageAndAllReference() 可以完全不用考虑错误处理

使用异常替代错误码的第二个好处是完全遵循了 OCP 开放关闭原则。项目中经常见到 ErrorCode/ReturnCode/Result 这样的 enum 类型,很多的类都依赖它。每次修改时,所有依赖的代码都需要重新编译和部署,因此很多程序员宁愿复用现有的、不太精确的错误码(如 UNKNOWN_ERROR),也不愿增加一个更清晰、准确的错误码。而异常具有继承体系,可以通过增加新的子类来扩展错误,而完全不用担心影响旧的代码。

总结一下,用异常替代错误码的好处:

  1. 使用户能够将错误处理的代码从主要逻辑中分离出来,阅读代码时能够轻松跳过错误处理部分,将重点放在主要逻辑
  2. 遵循开放关闭原则(OCP),利用异常的继承体系,新的异常可以作为 exception 的新增子类,旧的代码不需要改动

⚠️ 虽然异常有很多优点,但抛出异常会产生额外的开销(可能影响到确定性调度)。此外,编写异常安全的代码非常困难!因此一些对安全有特殊要求的应用会禁止使用异常,如汽车领域中很多对功能安全有要求的场合会禁止使用异常。

10. 不要重复(DRY: Don't Repeat Yourself)

重复是一切邪恶的根源。许多原则和规则都是为了消除重复:Codd 数据库范式、面向对象的继承、函数……

11. 结构化编程

之前的《架构整洁之道》中有介绍过,结构化编程是三个编程范式之一。所谓结构化编程,即每个函数、代码块都应该只有一个入口、一个出口。具体对编码的限制是:一个函数只有一个 return,循环中不能有 break/continue,绝对不能使用 goto。

作者认可结构化编程的目标和规范,但是!!!这些规则对于小函数帮助不大。如果函数足够短小,及早地 return/break/continue 可以使代码意图更清晰!而 goto 通常只有在大函数中才有作用,小函数中应该避免使用。

12. 如何写出这样的函数

即使作者 Uncle Bob 也不能一开始就写出这样的函数。

  • 先写初稿,并配上单元测试
  • 重构、优化:拆解函数、消除重复、修改名称
  • 保持单元测试通过