更优雅的LINQ替代Foreach扩展

Question

这纯粹是为了提高我的技能.我的解决方案适用于主要任务,但它不是"整洁".我目前正在使用Entity框架项目开发.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
}

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

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();

为什么在地球ToList上Users？那看起来不对.

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;

已经好多了.

ForEach如果你只是要通过一个lambda身体,切勿使用它作为一种方法!语言中已经有一个内置的机制可以做得更好!items.Foreach(item => {...});你可以简单地写一下,没有理由写foreach(var item in items) { ... }.它更简单,更容易理解和调试,编译器可以更好地优化它.

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

好多了.

为什么会出现ToList上Servers？完全没必要!

    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)" 
  };

现在我们的道具体是

    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中,您可以使用元组而不是匿名类型,但这个想法是相同的.)