更优雅的 LINQ 替代 Foreach 扩展

Question

这纯粹是为了提高我的技能。我的解决方案适用于主要任务，但它并不“整洁”。我目前正在开发一个带有实体框架项目的 .NET MVC。我只知道多年来已经足够的基本奇异 LINQ 函数。现在我想学习如何幻想。

所以我有两个模型

public class Server
{
    [Key]
    public int Id { get; set; }
    public string InstanceCode { get; set; }
    public string ServerName { get; set; }
}

public class Users
{
    [Key]
    public int Id { get; set; }
    public string Name { get; set; }
    public int ServerId { get; set; } //foreign key relationship
}

在我的一个视图模型中，我被要求提供一个下拉列表，用于在创建新用户时选择服务器。以 IEnumerable 形式填充文本和值 Id 的下拉列表 这是我的服务器下拉列表的原始属性

public IEnumerable<SelectListItem> ServerItems
{
    get { Servers.ToList().Select(s => new selectListItem { Value = x.Id.ToString(), Text = $"{s.InstanceCode}@{s.ServerName}" }); }
}

更新要求，现在我需要显示与每个服务器选择相关的用户数。好的，没问题。这是我从头顶写下的内容。

public IEnumerable<SelectListItem> ServerItems
{
    get 
    {
        var items = new List<SelectListItem>();
        Servers.ToList().ForEach(x => {
            var count = Users.ToList().Where(t => t.ServerId == x.Id).Count();
            items.Add(new SelectListItem { Value = x.Id.ToString(), Text = $"{x.InstanceCode}@{x.ServerName} ({count} users on)" });
        });

        return items;
    }
}

这得到我的结果，可以说“localhost@rvrmt1u（8 个用户）”，但就是这样.. 如果我想按用户数对这个下拉列表进行排序怎么办。我所做的只是字符串中的另一个变量。

TLDR ...我确信某处的某个人可以教我一两件事，将其转换为 LINQ 查询并使其看起来更好。还知道如何对列表进行排序以首先显示用户最多的服务器。

Answer 1

好的，我们有这个烂摊子：

    var items = new List<SelectListItem>();
    Servers.ToList().ForEach(x => {
        var count = Users.ToList().Where(t => t.ServerId == x.Id).Count();
        items.Add(new SelectListItem { Value = x.Id.ToString(), Text = $"{x.InstanceCode}@{x.ServerName} ({count} users on)" });
    });
    return items;

进行一系列小的、仔细的、明显正确的重构，逐步改进代码。

开始：让我们将那些复杂的操作抽象为他们自己的方法。

请注意，我已将无用的 x 替换为有用的 server。

int UserCount(Server server) => 
  Users.ToList().Where(t => t.ServerId == server.Id).Count();

为什么Users 上有一个ToList？看起来不对。

int UserCount(Server server) => 
  Users.Where(t => t.ServerId == server.Id).Count();

我们注意到有一个内置方法可以同时执行这两个操作：

int UserCount(Server server) => 
  Users.Count(t => t.ServerId == server.Id);

同样用于创建项目：

SelectListItem CreateItem(Server server, int count) => 
  new SelectListItem 
  { 
    Value = server.Id.ToString(), 
    Text = $"{server.InstanceCode}@{server.ServerName} ({count} users on)" 
  };

现在我们的属性体是：

    var items = new List<SelectListItem>();
    Servers.ToList().ForEach(server => 
    {
        var count = UserCount(server);
        items.Add(CreateItem(server, count);
    });
    return items;

已经好多了。

如果你只是要传递一个 lambda 主体，千万不要使用 ForEach 作为方法！语言中已经有一个内置机制可以做得更好！当您可以简单地写foreach(var item in items) { ... } 时，没有理由写items.Foreach(item => {...});。更简单更容易理解和调试，编译器可以更好的优化。

    var items = new List<SelectListItem>();
    foreach (var server in Servers.ToList())
    {
        var count = UserCount(server);
        items.Add(CreateItem(server, count);
    }
    return items;

好多了。

为什么Servers 上有一个ToList？完全没必要！

    var items = new List<SelectListItem>();
    foreach(var server in Servers)
    {
        var count = UserCount(server);
        items.Add(CreateItem(server, count);
    }
    return items;

越来越好。我们可以消除不必要的变量。

    var items = new List<SelectListItem>();
    foreach(var server in Servers)
        items.Add(CreateItem(server, UserCount(server));
    return items;

嗯。这让我们了解到CreateItem 可能会自己进行计数。让我们重写它。

SelectListItem CreateItem(Server server) => 
  new SelectListItem 
  { 
    Value = server.Id.ToString(), 
    Text = $"{server.InstanceCode}@{server.ServerName} ({UserCount(server)} users on)" 
  };

现在我们的 prop body 是

    var items = new List<SelectListItem>();
    foreach(var server in Servers)
        items.Add(CreateItem(server);
    return items;

这应该看起来很熟悉。我们重新发明了Select 和ToList：

var items = Servers.Select(server => CreateItem(server)).ToList();

现在我们注意到 lambda 可以替换为方法组：

var items = Servers.Select(CreateItem).ToList();

我们已将整个混乱简化为一行，清晰明确地看起来像它所做的那样。它有什么作用？它为每个服务器创建一个项目并将它们放在一个列表中。 代码应该读起来像它的作用，而不是它的作用。

仔细研究我在这里使用的技术。

• 将复杂代码提取到辅助方法中
• 用真实循环替换ForEach
• 消除不必要的ToLists
• 当您意识到需要改进时，重新审视之前的决定
• 在重新实现简单的辅助方法时进行识别
• 不要只做一项改进！每一项改进都可以使另一项改进成为可能。

---

如果我想按用户数对这个下拉列表进行排序怎么办？

然后按用户数排序！我们将其抽象为一个辅助方法，因此我们可以使用它：

var items = Servers
  .OrderBy(UserCount)
  .Select(CreateItem)
  .ToList();

我们现在注意到我们调用了UserCount两次。我们在乎吗？也许。调用两次可能是一个性能问题，或者，可怕的是，它可能不是幂等的！如果其中任何一个有问题，那么我们需要撤销我们之前做出的决定。在理解模式下处理这种情况比在流利模式下更容易，所以让我们重写为理解：

var query = from server in Servers
            orderby UserCount(server)
            select CreateItem(server);
var items = query.ToList();

现在我们回到之前的：

SelectListItem CreateItem(Server server, int count) => ...

现在我们可以说

var query = from server in Servers
            let count = UserCount(server)
            orderby count
            select CreateItem(server, count);
var items = query.ToList();

我们只在每台服务器上调用一次UserCount。

为什么要回到理解模式？因为在流利模式下这样做会造成混乱：

var query = Servers
  .Select(server => new { server, count = UserCount(server) })
  .OrderBy(pair => pair.count)
  .Select(pair => CreateItem(pair.server, pair.count))
  .ToList();

而且看起来有点难看。 （在 C# 7 中，您可以使用元组而不是匿名类型，但想法是一样的。）

Answer 2

LINQ 的诀窍就是输入return 并从那里开始。不要创建列表并向其中添加项目；通常有一种方法可以一次性选择所有内容。

public IEnumerable<SelectListItem> ServerItems
{
    get 
    {
        return Servers.Select
        (
            server => 
            new 
            {
                Server = server,
                UserCount = Users.Count( u => u.ServerId = server.Id )
            }
        )
        .Select
        (
            item =>
            new SelectListItem
            {
                Value = item.Server.Id.ToString(),
                Text = string.Format
                (
                    @"{0}{1} ({2} users on)" ,
                    item.Server.InstanceCode,
                    item.Server.ServerName, 
                    item.UserCount
                )
            }
        );
    }
}

在这个例子中，实际上有两个Select 语句——一个用于提取数据，一个用于格式化。在理想情况下，这两个任务的逻辑将被分成不同的层，但这是一个不错的折衷方案。