Enumerable.Single 的错误实现？

Question

我在 Enumerable.cs 中通过反射器发现了这个实现。

public static TSource Single<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
    //check parameters
    TSource local = default(TSource);
    long num = 0L;
    foreach (TSource local2 in source)
    {
        if (predicate(local2))
        {
            local = local2;
            num += 1L;
            //I think they should do something here like:
            //if (num >= 2L) throw Error.MoreThanOneMatch();
            //no necessary to continue
        }
    }
    //return different results by num's value
}

我认为如果有两个以上的项目满足条件，他们应该打破循环，为什么他们总是循环整个集合？万一那个反射器错误地反汇编了dll，我写了一个简单的测试：

class DataItem
{
   private int _num;
   public DataItem(int num)
   {
      _num = num;
   }

   public int Num
   {
      get{ Console.WriteLine("getting "+_num); return _num;}
   }
} 
var source = Enumerable.Range(1,10).Select( x => new DataItem(x));
var result = source.Single(x => x.Num < 5);

对于这个测试用例，我认为它会打印“getting 0, getting 1”然后抛出异常。但事实是，它一直在“得到 0...得到 10”并抛出异常。 他们这样实现这个方法有什么算法原因吗？

EDIT你们中的一些人认为是因为谓词表达式的副作用，经过深思熟虑和一些测试用例，我有一个结论side在这种情况下效果并不重要。如果您不同意这个结论，请举例说明。

Answer 1

是的，我确实觉得这有点奇怪，特别是因为不带谓词的重载（即仅适用于序列）确实似乎具有快速“优化”。

---

然而，在 BCL 的辩护中，我会说 Single 抛出的 InvalidOperation 异常是 boneheaded exception，通常不应将其用于控制​​流。 对于这种情况，没有必要由库优化。

使用Single 的代码，其中零个或多个匹配是完全有效的可能性，例如：

try
{
     var item = source.Single(predicate);
     DoSomething(item);
}

catch(InvalidOperationException)
{
     DoSomethingElseUnexceptional();    
}

应该重构为不对控制流使用异常的代码，例如（只是一个示例；这可以更有效地实现）：

var firstTwo = source.Where(predicate).Take(2).ToArray();

if(firstTwo.Length == 1) 
{
    // Note that this won't fail. If it does, this code has a bug.
    DoSomething(firstTwo.Single()); 
}
else
{
    DoSomethingElseUnexceptional();
}

---

换句话说，我们应该将Single 的使用留给我们希望序列包含仅一个匹配项的情况。它的行为应该与First 相同，但带有附加的运行时断言，即该序列不包含多个匹配项。与任何其他断言一样，失败，即Single 抛出的情况，应该用于表示程序中的错误（在运行查询的方法中或在调用者传递给它的参数中） .

这给我们留下了两种情况：

1. 断言成立：只有一个匹配项。在这种情况下，我们希望Single 消耗整个序列无论如何 来断言我们的声明。 “优化”没有任何好处。事实上，有人可能会争辩说，由 OP 提供的“优化”的示例实现实际上会更慢，因为对循环的每次迭代都进行了检查。
2. 断言失败：有零个或多个匹配项。在这种情况下，我们确实抛出比我们可以晚，但这并不是什么大不了的，因为异常是愚蠢的：它表明存在错误必须修复。

总而言之，如果“糟糕的实现”在生产中影响了您的性能，那么：

1. 您错误地使用了Single。
2. 您的您的程序中有错误。修复错误后，这个特定的性能问题就会消失。

编辑：澄清我的观点。

编辑：这是 Single 的 有效 用法，其中失败表示 调用 代码中的错误（错误参数）：

public static User GetUserById(this IEnumerable<User> users, string id)
{
     if(users == null)
        throw new ArgumentNullException("users");

     // Perfectly fine if documented that a failure in the query
     // is treated as an exceptional circumstance. Caller's job 
     // to guarantee pre-condition.        
     return users.Single(user => user.Id == id);    
}

Answer 2

更新：
我的回答得到了一些非常好的反馈，这让我重新思考。因此，我将首先提供陈述我的“新”观点的答案；您仍然可以在下面找到我的原始答案。请务必阅读中间的 cmets，以了解我的第一个答案在哪里漏掉了重点。

新答案：

假设Single 应该在不满足前提条件时抛出异常；也就是说，当Single 检测到没有或集合中的多个项目与谓词匹配时。

Single只有通过遍历整个集合才能成功而不抛出异常。它必须确保只有一个匹配项，因此必须检查集合中的所有项。

这意味着尽早抛出异常（一旦找到第二个匹配项）本质上是一种优化，只有当Single的前置条件无法满足并且它会抛出异常时才能受益.

正如用户 CodeInChaos 在下面的评论中明确指出的那样，优化不会错，但毫无意义，因为通常会引入有益于正常工作代码的优化，而不是有益于故障代码的优化。

因此，Single 可以提前抛出异常实际上是正确的；但它不是必须的，因为几乎没有额外的好处。

---

旧答案：

我无法给出技术原因为什么该方法按原样实现，因为我没有实现它。但我可以说明我对Single 运算符的目的的理解，并由此得出我个人的结论，即它确实执行得很差：

我对@9​​87654327@的理解：

Single 的目的是什么，它与 e.g. 有何不同？ First 或 Last?

使用Single 运算符基本上表达了一个假设，即必须从集合中返回一个项目：

• 如果不指定谓词，则应表示该集合应仅包含一项。

• 如果您确实指定了一个谓词，这应该意味着集合中的一个项目应该满足该条件。 （使用谓词应该与items.Where(predicate).Single() 具有相同的效果。）

这就是Single 与First、Last 或Take(1) 等其他运算符的不同之处。这些运营商都没有要求恰好有一个（匹配的）项目。

Single 什么时候应该抛出异常？

基本上，当它发现你的假设是错误的；即当底层集合确实 not 产生一个（匹配的）项目时。也就是说，当有零个或多个项目时。

什么时候应该使用Single？

Single 的使用适用于当您的程序逻辑可以保证集合将恰好产生一项且仅产生一项时。如果抛出异常，那应该意味着您的程序逻辑包含错误。

如果您处理“不可靠”的集合，例如 I/O 输入，则应先验证输入，然后再将其传递给 Single。 Single 以及异常 catch 块不适合确保集合只有一个匹配项。当您调用 Single 时，您应该已经确保只有一个匹配项。

结论：

以上陈述了我对Single LINQ 运算符的理解。如果您遵循并同意这种理解，您应该得出的结论是Single 应该尽早抛出异常。没有理由等到（可能非常大的）集合结束，因为Single 的前提条件一旦检测到集合中的第二个（匹配的）项目就被违反了。

Answer 3

在考虑这个实现时，我们必须记住这是 BCL：应该在各种情况下都可以正常工作的通用代码足够。

首先，采取这些场景：

1. 迭代 10 个数字，其中第一个和第二个元素相等
2. 迭代超过 1.000.000 个数字，其中第一个和第三个元素相等

原始算法对于 10 个项目来说足够好，但是 1M 会严重浪费周期。因此，在这些情况下我们知道序列中有两个或多个早期，建议的优化会产生很好的效果。

然后，看看这些场景：

1. 迭代超过 10 个数字，其中第一个和最后一个元素相等
2. 迭代超过 1.000.000 个数字，其中第一个和最后一个元素相等

在这些情况下，算法仍然需要检查列表中的每个项目。没有捷径可走。原始算法将执行良好足够，它履行合同。更改算法，在每次迭代中引入if 实际上会降低 性能。对于 10 个项目，它可以忽略不计，但 100 万个它会很受欢迎。

IMO，最初的实现是正确的，因为它对于大多数情况来说已经足够好了。了解Single 的实现虽然很好，因为它使我们能够根据我们对使用它的序列的了解做出明智的决策。如果在一个特定场景中的性能测量显示Single 导致了瓶颈，那么我们可以实现自己的变体，在该特定场景中效果更好。

更新：正如CodeInChaos和Eamon已经正确指出的那样，优化中引入的if测试确实没有对每个项目进行，只是在谓词匹配块内。在我的示例中，我完全忽略了这样一个事实，即提议的更改不会影响实现的整体性能。

我同意引入优化可能会使所有场景受益。很高兴看到最终实施优化的决定是基于性能测量。

Answer 4

我认为这是一个过早的优化“错误”。

由于副作用，为什么这是不合理的行为

有些人认为，由于副作用，应该对整个列表进行评估。毕竟，在正确的情况下（序列确实只有 1 个元素）它是完全枚举的，为了与这种正常情况保持一致，最好在 所有 情况下枚举整个序列。

虽然这是一个合理的论点，但它与整个 LINQ 库的一般做法背道而驰：它们到处都使用惰性求值。除非绝对必要，否则完全枚举序列是不的一般做法；实际上，有几种方法更喜欢使用IList.Count，即使在任何迭代都可用时 - 即使该迭代可能有副作用。

此外，.Single() 没有 谓词不会表现出这种行为：即尽快终止。如果参数是 .Single() 应该尊重枚举的副作用，那么您会期望所有重载都等效地这样做。

为什么速度的理由不成立

Peter Lillevold 提出了一个有趣的观察，即这样做可能更快......

foreach(var elem in elems)
    if(pred(elem)) {
        retval=elem;
        count++;
    }
if(count!=1)...

比

foreach(var elem in elems)
    if(pred(elem)) {
        retval=elem;
        count++;
        if(count>1) ...
    }
if(count==0)...

毕竟，一旦检测到第一个冲突，就会退出迭代的第二个版本需要在循环中进行额外的测试——“正确”中的测试纯粹是镇流器。简洁的理论，对吧？

除了，这不是由数字决定的；例如在我的机器上 (YMMV) Enumerable.Range(0,100000000).Where(x=>x==123).Single() 实际上快 比 Enumerable.Range(0,100000000).Single(x=>x==123)!

这可能是这台机器上这种精确表达式的 JITter 怪癖 - 我并不是说 Where 后跟无谓词 Single 总是更快。

但无论如何，快速失败的解决方案不太可能明显变慢。毕竟，即使在正常情况下，我们正在处理一个廉价的分支：一个从不被采用的分支，因此在分支预测器上很容易。而且当然;只有当 pred 持有时才会进一步遇到分支 - 在正常情况下每次调用一次。与委托调用 pred 及其实现的成本相比，该成本可以忽略不计，加上接口方法 .MoveNext() 和 .get_Current() 及其实现的成本。

与所有其他抽象损失相比，您几乎不可能注意到由一个可预测分支引起的性能下降 - 更不用说大多数序列和谓词实际上自己做某事这一事实。

Answer 5

对我来说似乎很清楚。

Single 适用于调用者知道枚举只包含一个匹配项的情况，因为在任何其他情况下都会引发昂贵的异常。

对于这个用例，接受谓词的重载必须遍历整个枚举。在每个循环上没有附加条件的情况下这样做会稍微快一些。

在我看来，当前的实现是正确的：它针对包含恰好一个匹配元素的枚举的预期用例进行了优化。

Answer 6

在我看来，这似乎是一个糟糕的实现。

只是为了说明问题的潜在严重性：

var oneMillion = Enumerable.Range(1, 1000000)
                           .Select(x => { Console.WriteLine(x); return x; });

int firstEven = oneMillion.Single(x => x % 2 == 0);

上面会在抛出异常之前输出1到1000000的所有整数。

这肯定令人头疼。

Answer 7

我在https://connect.microsoft.com/VisualStudio/feedback/details/810457/public-static-tsource-single-tsource-this-ienumerable-tsource-source-func-tsource-bool-predicate-doesnt-throw-immediately-on-second-matching-result#提交报告后才发现这个问题

副作用论点不成立，因为：

1. 有副作用并不是真正的功能，它们被称为Func 是有原因的。
2. 如果您确实需要副作用，那么声明在整个序列中具有副作用的版本是可取的，而不是声明立即抛出的版本是可取的。
3. 它与First 的行为或Single 的其他重载不匹配。
4. 它至少与Single 的一些其他实现不匹配，例如Linq2SQL 使用TOP 2 确保只返回测试多个匹配项所需的两个匹配案例。
5. 我们可以构建预期程序会停止但并未停止的情况。
6. 我们可以构建抛出 OverflowException 的案例，这不是记录的行为，因此显然是一个错误。

最重要的是，如果我们处于预期序列只有一个匹配元素的情况下，但事实并非如此，那么显然出现了问题。除了在检测到错误状态时唯一应该做的事情是在抛出之前进行清理（并且此实现会延迟）这一一般原则之外，具有多个匹配元素的序列的情况将与以下情况重叠具有比预期更多的元素的序列 - 可能是因为该序列有一个错误导致它意外循环。因此，正是在应该触发异常的一组可能的错误中，异常被延迟最多。

编辑：

Peter Lillevold 提到重复测试可能是作者选择采用他们所做的方法的原因，作为对非异常情况的优化。如果是这样，那就没必要了，即使除了 Eamon Nerbonne 的表现，它也不会有太大的改善。在初始循环中无需重复测试，因为我们可以在第一次匹配时更改我们正在测试的内容：

public static TSource Single<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
  if(source == null)
    throw new ArgumentNullException("source");
  if(predicate == null)
    throw new ArgumentNullException("predicate");
  using(IEnumerator<TSource> en = source.GetEnumerator())
  {
    while(en.MoveNext())
    {
      TSource cur = en.Current;
      if(predicate(cur))
      {
        while(en.MoveNext())
          if(predicate(en.Current))
            throw new InvalidOperationException("Sequence contains more than one matching element");
       return cur;
      }
    }
  }
  throw new InvalidOperationException("Sequence contains no matching element");
}