记一次C#代码审查

  var results = (from ch in db.CookiesHistory where ch.UserId == myCookie.ID select new {     GitUserId = ch.GitUserId    });    foreach(var result in results) {     var user = (from u in db.GitUsers where u.Id == result.GitUserId select new {      u     }).First();     favoritesList.Add(user.u);   return favoritesList; 

看起来是很普通的代码。从哪儿开始呢? 上下文! 上下文是重点。这很明显。但首先，你需要提出以下问题：

- 你想在这里实现什么?

- 有什么要求?

这是启动每个代码审查过程的最佳方式。一要求做代码审查的人已经知道这些问题的答案。 此函数返回一个 GitHub 收藏的用户列表。 在 GitHub 上，有一个追踪用户的能力。

初步意见：

- 我看到 Git，但没有GitHub API客户端调用。

- LINQ 表示我们正在做一些查询。

- Context 关键字是实体框架(ORM 访问数据)。

- HttpContext 和 Cookies 在 ASP.NET 范围内执行。

- 没有参数的函数返回收藏夹列表。

问题：

- 在哪里调用 GitHub API? 这是一个存储在外部数据存储区中的 GitHub 数据的快照。

命名约定

理想的代码是易于理解的代码。我确实偏爱 KISS 的原则; 这是一个务实的做法。 一个简单而优雅的代码胜利一切。 我希望读一份代码就像读一本叙述书。 现实中一定也存在某种机制，可以使我们的代码更好读。让我们从函数名开始。

public IEnumerable GitHubUser  FavoritesList() 

这还不够。它没有告诉我任何信息。

public IEnumerable GitHubUser  FavouriteGitHubUsers() 

好点了……不过 GitHub 上并没有 favorites 的概念。只有一个 followed users 的概念。

public IEnumerable GitHubUser  FollowedGitHubUsers() 

正确的命名函数并把它作为商务语言的简单解释是很重要滴。 领域驱动设计专家可能会称之”通俗易懂“的沟通。

“收藏”这个词会使混迹 GitHub 的老鸟把它和其他东西混淆。这种混淆会造成时间的浪费，换言之，就是浪费金钱。不要去创造新的说法，当你需要“引入”新词或新说法时，多讨论下，问问该领域的专家和小伙伴们。

或许已经有一个适合该函数的名称或者些许代码。像我们号召的一样，力求创造简短的代码，我们还应该保证我们领域词汇的统一，简洁，以及通俗易懂。

无参的函数

对我来说，没有参数的函数是一个反面模式。这可能在某些情况下有用，但这些情况非常罕见，主要与已经不完整的设计架构有关。

如果函数没有任何参数，那么它有什么作用呢? 它从哪里获得输入和当前状态呢? 它可能是一个全局的 - 但这也是一个反面模式。函数需要纯输入和纯输出。

纯函数是一个很好的概念; 这是我在探索函数式编程时学到的。

纯函数是一个这样的函数：其中返回值仅由其输入值确定，而没有其他可观察到的副作用。这是在数学中的函数如何工作的原理。对于相同的 x 值，Math.cos(x)将始终返回相同的结果。

由于 cookie 中的全局状态和对 HTTPContext 的依赖，当前函数不是纯函数。我朋友曾尝试将该逻辑隐藏在 CookieHelper 中来访问它们，但这还不够。 问题依然存在。

在 Asp.NET 中，在函数之外使用 HTTPContext。它不应该泄漏到里面。当前代码不能在不同的上下文中工作。如果我们在没有全局的 HTTPContext 的进程中执行它，它将崩溃。

这里还有另一个反模式，新的关键字。尝试注入尽可能多的东西。 这个建议可能被不正确地使用，但依赖仍不容忽视。

仔细看看代码，其中包含 CookieHelper 允许访问 myCookie.ID 的逻辑。 我们可以通过引入一个包含参数 ID 的函数来从这个函数中移除所有这些逻辑。

public IEnumerable GitHubUser  FollowedGitHubUsers(int cookieId) 

我们不在乎 cookieId 的值来自哪里。这给出了更多的选择，因为我们可以从其他源获取 cookieId，而不仅仅是从 cookie 中。

更改后的代码

将代码划分为独立的功能

有两个 DB 查询，都使用同样的 Context。 有如下代码：

代码是不是看起来既简单又美观? 所有这一切，归功于功能的提取，使得代码非常容易阅读。在这个层次上，我不在乎如何获得用户。 我只需要专注一些功能和输入的编排。

如果我需要了解更多的细节，我才会进入这个方法查看。

private IEnumerable GitHubUser FindUsers(GitHubContext db, IEnumerable int  userIds)          List GitHubUser  favoritesList = new List GitHubUser           foreach (var result in results)          {              var user = (from u in db.GitUsers                          where u.Id == result.GitUserId                          select new { u }).First();              favoritesList.Add(user.u);          }      }      return favoritesList; 

运用这个函数也让我能更好地理解它。这里的输入和输出非常明确。简单、短小的函数可以更加清晰地帮助大脑消化信息。而使用较小的函数，我们必须考虑代码行数量较少的情况。

拥有明确的输入和输出，我们就可以界定范围和上下文。 这里没有 cookie helper。我们不用担心从哪里获取 userId 的问题。我们避开它的复杂性，而把精力放在重要的问题上。 这样能有效避免函数中的干扰项。

作者：OSC-协作翻译

来源：51CTO

如何提高代码质量 说起代码质量，脑子里会冒出很多词，命名规范、格式规范、日志规范、单元测试覆盖率... 但我觉得，代码质量总结起来就两个：好看和好用。 好看是指代码可读性好，容易理解、容易维护，别人接手了不骂你；好用则指代码健壮，不容易出错，机器跑着不骂你。即使出错，也容易定位，容易止损和恢复。