发现代码质量问题 - 常规Checklist

• 目录设置是否合理、模块划分是否清晰、代码结构是否满足“高内聚、松耦合”？
• 是否遵循经典的设计原则和设计思想（SOLID、DRY、KISS、YAGNI、LOD 等）？
• 设计模式是否应用得当？是否有过度设计？
• 代码是否容易扩展？如果要添加新功能，是否容易实现？
• 代码是否可以复用？是否可以复用已有的项目代码或类库？是否有重复造轮子？
• 代码是否容易测试？单元测试是否全面覆盖了各种正常和异常的情况？
• 代码是否易读？是否符合编码规范（比如命名和注释是否恰当、代码风格是否一致等）？

发现代码质量问题 - 业务Checklist

• 代码是否实现了预期的业务需求？
• 逻辑是否正确？是否处理了各种异常情况？
• 日志打印是否得当？是否方便 debug 排查问题？
• 接口是否易用？是否支持幂等、事务等？
• 代码是否存在并发问题？是否线程安全？
• 性能是否有优化空间，比如，SQL、算法是否可以优化？
• 是否有安全漏洞？比如输入输出校验是否全面？

重构步骤

• 第一轮重构：提高代码的可读性
• 第二轮重构：提高代码的可测试性
• 第三轮重构：编写完善的单元测试
• 第四轮重构：所有重构完成之后添加注释

程序出错返回的最佳实践

• 返回错误码

  如果你熟悉的编程语言中有异常这种语法机制，那就尽量不要使用错误码。异常相对于错误码，有诸多方面的优势，比如可以携带更多的错误信息（exception 中可以有 message、stack trace 等信息）等。

• 返回 NULL 值

  缺点：

  • 易导致 空指针异常 。如果某个函数有可能返回 NULL 值，我们在使用它的时候，忘记了做 NULL 值判断，就有可能会抛出空指针异常。
  • 影响代码的可读性。如果我们定义了很多返回值可能为 NULL 的函数，那代码中就会充斥着大量的 NULL 值判断逻辑，一方面写起来比较繁琐，另一方面它们跟正常的业务逻辑耦合在一起，会影响代码的可读性。

  最佳实践：

  • 对于以 get、find、select、search、query 等单词开头的查找函数来说，数据不存在，并非一种异常情况，这是一种正常行为。所以，返回代表不存在语义的 NULL 值比返回异常更加合理。

• 返回空对象

  应用空对象设计模式（Null Object Design Pattern），可以避免返回 NULL 的缺点。
  当函数返回的数据是字符串类型或者集合类型的时候，我们可以用空字符串或空集合替代 NULL 值，来表示不存在的情况。这样，我们在使用函数的时候，就可以不用做 NULL 值判断。

• 抛出异常对象

  • 运行时异常（Runtime Exception）

    也叫作非受检异常（Unchecked Exception）。对于代码 bug（比如数组越界）以及不可恢复异常（比如数据库连接失败），即便我们捕获了，也做不了太多事情，所以，我们倾向于使用非受检异常。
    新增非受检异常可以不改动调用链上的代码。我们可以灵活地选择在某个函数中集中处理，比如在 Spring 中的 AOP 切面中集中处理异常。

    缺点：

    • 非受检异常使用起来更加灵活，怎么处理的主动权这里就交给了程序员。过于灵活会带来不可控，非受检异常不需要强制捕获处理，那程序员就有可能漏掉一些本应该捕获处理的异常。

  • 编译时异常（Compile Exception）

    也叫作受检异常（Checked Exception）。对于可恢复异常、业务异常，比如提现金额大于余额的异常，我们更倾向于使用受检异常，明确告知调用者需要捕获处理。

    缺点：

    • 影响代码的可读性。受检异常需要显式地在函数定义中声明。如果函数会抛出很多受检异常，那函数的定义就会非常冗长，这就会影响代码的可读性，使用起来也不方便。
    • 代码实现会比较繁琐。编译器强制我们必须显示地捕获所有的受检异常，代码实现会比较繁琐。
    • 违反开闭原则。如果我们给某个函数新增一个受检异常，这个函数所在的函数调用链上的所有位于其之上的函数都需要做相应的代码修改，直到调用链中的某个函数将这个新增的异常 try-catch 处理掉为止。

  • 处理异常的方式

    • 直接吞掉。
    • 原封不动地 re-throw。
    • 包装成新的异常 re-throw。

  异常是否往上继续抛出，要看上层代码是否关心这个异常。关心就将它抛出，否则就直接吞掉。是否需要包装成新的异常抛出，看上层代码是否能理解这个异常、是否业务相关。如果能理解、业务相关就可以直接抛出，否则就封装成新的异常抛出。