代码审查关注什么：SOLID 原则

在今天的文章中，我们将更仔细的讨论代码本身的设计，特别检查是否遵循了良好的面向对象设计实践。和我们已经讨论过的其他方面一样，不是所有的团队都会将 SOLID 原则列为最重要的检查项，但是如果你在尝试遵循 SOLID 原则，或者在尝试将你的代码往这方面发展，这里有一些提示可能对你有帮助。

SOLID 是什么？

SOLID 原则是面向对象设计和编程的5个核心原则。本文的目的不是详细讲解 SOLID 原则是什么或者深入讨论为什么你要遵循这些原则，而是指出在代码审查中怎么发现没有遵循这些原则的味道。

SOLID 代表：

• S - 单一功能原则
• O - 开闭原则
• L - 里氏替换原则
• I - 接口分离原则
• D - 依赖反转原则

单一功能原则（SRP）

在修改一个类时永远都应该只有一个理由

这一点在单次代码审查时可能比较难发现。根据这个规则的定义，作者在修改代码是有（或者应该有）一个理由--解决 bug，添加一个新功能，代码重构。

你需要关注一个类里面哪些方法可能会同时修改，以及哪些方法不会因为其他方法的修改而修改。例如：

通过 Upsource 的两栏差异比较会发现 TweetMonitor 中添加了一个新功能，在一些用户界面的发帖排行榜绘制前面10个发帖者的能力。这看起来是合理的，因为它使用了 onMessage 方法搜集好的数据，但是有迹象表明它破坏了 SRP 原则。OnMessage 和 getTweetMessageFromFullTweet 方法都是关于接收并解析一条 Twitter 消息，然而 draw 方法为了UI展示重新获取相关数据。

代码审查者应该标记出这两个职责，并且之后和作者一起讨论一个更好的方式来分割这两个功能：也许可以将 Twitter 字符串的解析移到一个不同的类中；或者创建一个不同的类来负责提供发帖排行榜。

开闭原则（OCP）

软件实体（类，模块，函数等等）应该对扩展开放，但是对修改封闭。

作为审查者，如果发现通过一系列的 if 语句来检查类型，你应该意识到破坏了开闭原则。

如果你在审查上面的代码，你应该很清楚的意识到，如果一种新的 Event 类型添加到系统中，那么新的类型创建者为了处理新添加的类型，它也许必须添加另一个 else 语句到这个方法中。

使用多态来替换这些 if 可能会好一些：

和往常一样，这个问题不止一个解决方法，但关键是将复杂的 if/else 和 instanceof 检查去掉。

里氏替换原则（LSP）

使用了基类引用的函数，在不知道基类子类的情况下，也能够使用子类的对象

发现破坏这一规则的简单方法就是关注显式的类型转换。如果你必须将一个对象转换为其他类型，那么你并没有“在不知道子类信息的情况下”使用基类。

在检查 LSP 的以下两个条件时，会发现更多微妙的破坏 LSP：

• （当子类的方法重载父类的方法时）方法的前置条件（即方法的形参）要比父类方法的输入参数更宽松。
• （当子类的方法实现父类的抽象方法时）方法的后置条件（即方法的返回值）要比父类更严格。

想象一下，例如我们有一个抽象类 Order，它有一系列子类 - BookOrder，ElectronicsOrder 等等。Order 类的
PlaceOrder 方法接收 Warehouse 参数，并以此修改仓库中的库存水平：

现在假设我们引入了新的电子礼品卡，这个只需要往钱包里添加余额就可以，不需要实际的库存。如果用 GiftCardOrder 类来实现电子礼品卡，placeOrder 方法就不必使用 warehouse 参数：

这看起来像是合理的使用继承，但事实上你是希望使用 GiftCardOrder 类的代码能够像使用其他类那样使用它，即你希望所有的子类都能通过测试：

但是这个测试并通不过，因为 GiftCardOrder 有不同的订购行为。如果你在审查这一类代码，确认这里使用继承是否合理--也许订购行为可以通过组合而不是继承来插入。

接口分离原则（ISP）

多个明确的客户端接口要好于一个通用的接口

如果代码中有接口定义了很多个方法，那么很容易确认它破坏了这一规则。这一条规则和 SRP 是一致的，你可能会发现拥有多个方法的接口实际上会负责多个方面或者功能。

但是有时只有两个方法的接口也应该分为两个接口：

在这个例子中，假设有时候不需要 decode 方法，并且某一个 codec 在不同的场合有可能当做 endoder 使用，有时可能当做 decoder 使用，那么把 SimpleCodec 拆分成 Encoder和 Decoder 更合适一些。有的类可能会同时实现这两个接口，但是不必让所有的实现者都去 Override 它们不需要的方法，或者说只需要 Encoder 接口的类注意到它们的 Encoder 实例还实现了 decode。

依赖反转原则（DIP）

依赖于抽象，而不是具体的实现。

发现简单的破坏这一规则可能比较容易，比如使用 new 关键字（而不是使用依赖注入或者工厂模式）或者对你的集合类型过度熟悉（例如将变量和参数定义为 ArrayList 而不是 List），作为审查者，你应该注意保证代码作者使用/创建了正确的抽象。

例如，服务级别的代码使用直接和数据库之间的连接来读写数据：

这段代码依赖于许多具体的实现细节：数据库连接 JDBC，数据库特定的 SQL，数据库的结构等等。这些代码应该出现在系统的某一个地方，但是不应该出现在这里，也不应该出现在不需要了解数据库细节的方法中。更好的方法是提取出一个 DAO 或者使用 Repository 模式，然后将 DAO 或者 repository 注入到这个 services。

总结

这些代码“味道”可能表示一个或者多个 SOLID 原则被破坏：

• 很长的 if/else 语句
• 强制转换到子类型
• 很多公共方法
• 实现了抛出 UnsupportedOperationException 的方法

与所有设计问题一样，在遵循这些原则之间找到平衡，并根据你的团队的喜好做出调整。 但是，如果在代码审查中看到复杂的代码，你可能会发现应用这些原则之一会找到一个更简单，更易于理解的解决方案。