编程语言做了1000次Code Review

Code Review 是保证代码质量的重要手段。Steven Heidel 曾在 LinkedIn 负责 Code Review，他在本文总结了常见的代码问题并提出修改方案。

当我在 LinkedIn 工作时，工作的很大一部分内容是做 Code Review。在这个过程中，我发现一些人很容易犯的错误，于是把错误整理起来并分享给团队。

经验 1：当出现错误时 Throw an exception

我看到的一个常见模式是：

1. List<String> getSearchResults(...) { 
2.   try { 
3.     List<String> results = // make REST call to search service 
4.     return results; 
5.   } catch (RemoteInvocationException e) { 
6.     return Collections.emptyList(); 
7.   } 
8. } 

上面的方法可能是很多新手工程师的做法，但这种模式会有问题。在我曾经参与的移动应用中，这种模式导致移动应用程序的故障。用户搜索开始后，我们的后端发生错误开始 throwing exceptions，但在应用程序的 API server 中并沒有 throwing exceptions。

因此，从应用角度看，前端会收到 200 个成功的响应，然后显示空白的搜索结果给使用者，而团队却毫不知情。

如果 API  thrown an exception，那我们的监控系统会立刻发现它，并能及时修复。

很多时候，当捕捉到异常后，我们倾向于返回 empty object。Java 中 empty object 的样例包括 Optional.empty()、null 和 empty list。这种情况经常发生在 URL 解析中。如果 URL 无法从字符串解析得到的话，不要返回 null，而要问问自己：

URL 格式为什么是不合法的？这是一个需要在 upstream 解决的数据问题吗？

对于这种任务来说，empty object 并不是恰当的工具。如果出现异常行为，那么就应该 throw an exception。

经验 2：尽可能使用最具体的类型（type)

基本而言，这条建议恰好与 stringly typed programming 相反。

我经常看到下面所示的代码：

1. void doOperation(String opType, Data data);  
2. // where opType is "insert", "append", or "delete", this should have clearly been an enum 
3.  
4. String fetchWebsite(String url); 
5. // where url is "https://google.com", this should have been an URN 
6.  
7. String parseId(Input input); 
8. // the return type is String but ids are actually Longs like "6345789" 

用最具体的类型 (type）可以避免很多 bug。

现在问题是：好心的程序员为什么会写出糟糕的 stringly typed 代码？

答案在于外部世界不是强类型的。字符串有很多不同的来源，比如：

• url 中的查询和路径参数
• JSON
• 不支持枚举的数据库
• 编写糟糕的库

在上述场景中，我们应使用如下的策略来避免该问题：将字符串解析和序列化放在程序的边缘之处。

下面是这样一个样例：

1. // Step 1: Take a query param representing a company name / member id pair and parse it 
2. // example: context=Pair(linkedin,456) 
3. Pair<String, Long> companyMember = parseQueryParam("context"); 
4. // this should throw an exception if malformed 
5.  
6. // Step 2: Do all the stuff in your application 
7. // MOST if not all of your code should live in this area 
8.  
9. // Step 3: Convert the parameter back into a String at the very end if necessary 
10. String redirectLink = serializeQueryParam("context"); 

这种方式有很多优点。立即发现格式错误的数据；如果出现任何问题，应用程序将提前 fails。数据被验证一次后，不必在整个应用程序中继续捕获解析异常。

此外，强类型使方法签名更具描述性，我们不再需要在每个方法上编写那么多的 javadocs。

经验 3：用 Optionals 而非 nulls

Java 8 带来最棒的特性之一是Optional类，它代表一个可能存在也可能不存在的实体。

一个小问题：

唯一拥有自己缩写的例外（exception）是什么？答案是 NPE 或空指针异常。截至目前，它是 Java 中最常见的异常，并被称为价值 10 亿美元的错误 (https://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare )。

Optional能让我们完全从程序中移除 NPE。但是，必须以正确的方式使用它。如下是关于使用Optional的一些建议：

• 我们不能在得到Optional的任何时候都简单地调用它的.get()，相反，我们要仔细考虑Optional不存在的情况并给出一个合理的默认值；
• 如果还没有合理的默认值，那么像.map()和.flatmap()这样的方法允许我们推迟到以后再做决定；
• 如果外部库返回null来表示为空的情况，那么立即使用Optional.ofNullable()wrap 该值。相信我，你以后会感谢自己的。null 值在程序内部有“bubble up”的倾向，所以最好在源代码中停止它们；
• 在方法的返回类型中使用Optional。这种做法非常好，因为我们不需要读取 javadoc 来确定值是否可能不存在。

额外建议：尽可能采用“Unlift”方法

我们应避免下面所示的方法：

1. // AVOID: 
2. CompletableFuture<T> method(CompletableFuture<S> param); 
3. // PREFER:  
4. T method(S param); 
5.  
6. // AVOID: 
7. List<T> method(List<S> param); 
8. // PREFER: 
9. T method(S param); 
10.  
11. // AVOID:  
12. T method(A param1, B param2, Optional<C> param3); 
13. // PREFER: 
14. T method(A param1, B param2, C param3); 
15. T method(A param1, B param2); 
16. // This method is clearly doing two things, it should be two methods 
17. // The same is true for boolean parameters 

上述不推荐使用的方法有哪些共同点？那就是它们都使用了 container objects 作为参数，比如 Optional、List 或 Task。

如果返回类型是相同种类的 container，那就更糟糕了（比如，param methods 接收 Optional，返回值也是 Optional）。

为什么呢？

1)Promise<A> method(Promise<B> param)要比 2)A method(B param)更缺少灵活性。

如果有一个Promise<B>的话，我们可以用 1)，也能通过.map函数使用 2)（即promise.map(method)）。

但是，如果只有一个 B 的话，我们很容易使用 2)，但是无法使用 1)，这样来看，2) 是更具灵活性的方案。

我喜欢将其称为“unlifting”，因为它与常见的函数式工具方法“lift”恰好相反。采用这种方式重写会让方法更具灵活性，对调用者更加易用。

在过去的几十年间，大量的编程语言被发明、被取代、被修改或组合在一起。尽管人们多次试图创造一种通用的程序设计语言，却没有一次尝试是成功的。之所以有那么多种不同的编程语言存在的原因是，编写程序的初衷其实也各不相同；新手与老手之间技术的差距非常大，而且有许多语言对新手来说太难学；还有，不同程序之间的运行成本（runtime cost）各不相同。