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

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

SOLID 是什么?

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

SOLID 代表:

单一功能原则(SRP)

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

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

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

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

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

开闭原则(OCP)

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

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

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

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

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

里氏替换原则(LSP)

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

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

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

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

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

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

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

接口分离原则(ISP)

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

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

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

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

依赖反转原则(DIP)

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

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

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

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

总结

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

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

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

本文译自: What to look for in a Code Review: SOLID Principles

推荐阅读更多精彩内容

  • 一:java概述:1,JDK:Java Development Kit,java的开发和运行环境,java的开发工...
    ZaneInTheSun阅读 2,283评论 0 11
  • 对象的创建与销毁 Item 1: 使用static工厂方法,而不是构造函数创建对象:仅仅是创建对象的方法,并非Fa...
    孙小磊阅读 1,590评论 0 3
  • 转载标注声明:http://www.uml.org.cn/sjms/201211023.asp 目录:[设计模式六...
    Bloo_m阅读 618评论 0 7
  • 1. Java基础部分 基础部分的顺序:基本语法,类相关的语法,内部类的语法,继承相关的语法,异常的语法,线程的语...
    子非鱼_t_阅读 30,484评论 18 399
  • 学而10:子禽问于子贡曰:“夫子至于是邦也,必闻其政。求之与?抑与之与?”子贡曰:“父子温、良、恭、俭、让以得之。...
    惊鸿掠波阅读 188评论 0 1
  • 今天和同事出差时聊到人生,听她提到张爱玲的一句话:生命是一袭华美的袍子,上面爬满了虱子。这句话的意思是,表面看起来...
    happycc阅读 163评论 0 0