Почему этот метод расширения строки не генерирует исключение?

119

У меня есть метод расширения строки C #, который должен возвращать IEnumerable<int>все индексы подстроки в строке. Он отлично работает по своему назначению, и ожидаемые результаты возвращаются (что доказано одним из моих тестов, хотя и не приведенным ниже), но другой модульный тест обнаружил проблему с ним: он не может обрабатывать нулевые аргументы.

Вот метод расширения, который я тестирую:

public static IEnumerable<int> AllIndexesOf(this string str, string searchText)
{
    if (searchText == null)
    {
        throw new ArgumentNullException("searchText");
    }
    for (int index = 0; ; index += searchText.Length)
    {
        index = str.IndexOf(searchText, index);
        if (index == -1)
            break;
        yield return index;
    }
}

Вот тест, который выявил проблему:

[TestMethod]
[ExpectedException(typeof(ArgumentNullException))]
public void Extensions_AllIndexesOf_HandlesNullArguments()
{
    string test = "a.b.c.d.e";
    test.AllIndexesOf(null);
}

Когда тест запускается с моим методом расширения, он терпит неудачу со стандартным сообщением об ошибке, что метод «не создал исключение».

Это сбивает с толку: я явно перешел nullв функцию, но по какой-то причине сравнение null == nullвозвращается false. Таким образом, исключение не создается, и код продолжается.

Я подтвердил, что это не ошибка теста: при запуске метода в моем основном проекте с вызовом Console.WriteLineв ifблоке сравнения с нулевым значением ничего не отображается на консоли, и ни один из catchдобавляемых блоков не перехватывает исключение . Кроме того, использование string.IsNullOrEmptyвместо == nullимеет ту же проблему.

Почему это якобы простое сравнение терпит неудачу?

ArtOfCode
источник
5
Вы пробовали пройти через код? Это, вероятно, решит проблему довольно быстро.
Мэтью Хоген,
1
Что же произошло? (Имеет ли он бросить в исключение, если это так, что один и какой линии?)
user2864740
@ user2864740 Я описал все, что происходит. Никаких исключений, только неудачный тест и метод запуска.
ArtOfCode
7
Итераторы не выполняются до тех пор, пока они не будут повторены
BlueRaja - Дэнни Пфлугофт
2
Пожалуйста. Этот также попал в список «худших ошибок» Джона: stackoverflow.com/a/241180/88656 . Это довольно частая проблема.
Эрик Липперт

Ответы:

158

Вы используете yield return. При этом компилятор перепишет ваш метод в функцию, которая возвращает сгенерированный класс, реализующий конечный автомат.

Вообще говоря, он переписывает локальные переменные в поля этого класса, и каждая часть вашего алгоритма между yield returnинструкциями становится состоянием. Вы можете проверить с помощью декомпилятора, чем становится этот метод после компиляции (обязательно отключите интеллектуальную декомпиляцию, которая будет производить yield return).

Но суть в следующем: код вашего метода не будет выполняться, пока вы не начнете итерацию.

Обычный способ проверки предварительных условий - разделить ваш метод на две части:

public static IEnumerable<int> AllIndexesOf(this string str, string searchText)
{
    if (str == null)
        throw new ArgumentNullException("str");
    if (searchText == null)
        throw new ArgumentNullException("searchText");

    return AllIndexesOfCore(str, searchText);
}

private static IEnumerable<int> AllIndexesOfCore(string str, string searchText)
{
    for (int index = 0; ; index += searchText.Length)
    {
        index = str.IndexOf(searchText, index);
        if (index == -1)
            break;
        yield return index;
    }
}

Это работает, потому что первый метод будет вести себя так, как вы ожидаете (немедленное выполнение), и вернет конечный автомат, реализованный вторым методом.

Обратите внимание, что вам также следует проверить strпараметр для null, потому что методы расширений могут вызываться для nullзначений, поскольку они просто синтаксический сахар.


Если вам интересно, что компилятор делает с вашим кодом, вот ваш метод, декомпилированный с помощью dotPeek с использованием параметра Показать код, созданный компилятором .

public static IEnumerable<int> AllIndexesOf(this string str, string searchText)
{
  Test.<AllIndexesOf>d__0 allIndexesOfD0 = new Test.<AllIndexesOf>d__0(-2);
  allIndexesOfD0.<>3__str = str;
  allIndexesOfD0.<>3__searchText = searchText;
  return (IEnumerable<int>) allIndexesOfD0;
}

[CompilerGenerated]
private sealed class <AllIndexesOf>d__0 : IEnumerable<int>, IEnumerable, IEnumerator<int>, IEnumerator, IDisposable
{
  private int <>2__current;
  private int <>1__state;
  private int <>l__initialThreadId;
  public string str;
  public string <>3__str;
  public string searchText;
  public string <>3__searchText;
  public int <index>5__1;

  int IEnumerator<int>.Current
  {
    [DebuggerHidden] get
    {
      return this.<>2__current;
    }
  }

  object IEnumerator.Current
  {
    [DebuggerHidden] get
    {
      return (object) this.<>2__current;
    }
  }

  [DebuggerHidden]
  public <AllIndexesOf>d__0(int <>1__state)
  {
    base..ctor();
    this.<>1__state = param0;
    this.<>l__initialThreadId = Environment.CurrentManagedThreadId;
  }

  [DebuggerHidden]
  IEnumerator<int> IEnumerable<int>.GetEnumerator()
  {
    Test.<AllIndexesOf>d__0 allIndexesOfD0;
    if (Environment.CurrentManagedThreadId == this.<>l__initialThreadId && this.<>1__state == -2)
    {
      this.<>1__state = 0;
      allIndexesOfD0 = this;
    }
    else
      allIndexesOfD0 = new Test.<AllIndexesOf>d__0(0);
    allIndexesOfD0.str = this.<>3__str;
    allIndexesOfD0.searchText = this.<>3__searchText;
    return (IEnumerator<int>) allIndexesOfD0;
  }

  [DebuggerHidden]
  IEnumerator IEnumerable.GetEnumerator()
  {
    return (IEnumerator) this.System.Collections.Generic.IEnumerable<System.Int32>.GetEnumerator();
  }

  bool IEnumerator.MoveNext()
  {
    switch (this.<>1__state)
    {
      case 0:
        this.<>1__state = -1;
        if (this.searchText == null)
          throw new ArgumentNullException("searchText");
        this.<index>5__1 = 0;
        break;
      case 1:
        this.<>1__state = -1;
        this.<index>5__1 += this.searchText.Length;
        break;
      default:
        return false;
    }
    this.<index>5__1 = this.str.IndexOf(this.searchText, this.<index>5__1);
    if (this.<index>5__1 != -1)
    {
      this.<>2__current = this.<index>5__1;
      this.<>1__state = 1;
      return true;
    }
    goto default;
  }

  [DebuggerHidden]
  void IEnumerator.Reset()
  {
    throw new NotSupportedException();
  }

  void IDisposable.Dispose()
  {
  }
}

Это недопустимый код C #, потому что компилятору разрешено делать то, что не разрешено языком, но разрешено в IL - например, именовать переменные таким образом, чтобы избежать конфликтов имен.

Но, как видите, AllIndexesOfonly создает и возвращает объект, конструктор которого только инициализирует некоторое состояние. GetEnumeratorтолько копирует объект. Настоящая работа выполняется, когда вы начинаете перечисление (путем вызова MoveNextметода).

Лукас Тшесневски
источник
9
Кстати, я добавил к ответу следующий важный момент: обратите внимание, что вам также следует проверить strпараметр null, потому что методы расширений могут вызываться для nullзначений, поскольку они просто синтаксический сахар.
Лукас Тшесневски
2
yield returnв принципе хорошая идея, но в ней есть так много странных ошибок. Спасибо, что осветили это!
nateirvin
Итак, в основном ошибка будет выдана, если перечислитель был запущен, как в foreach?
MVCDS
1
@MVCDS Именно так. MoveNextвызывается конструкцией под капотом foreach. Я написал объяснение того, что foreachв моем ответе объясняет семантику коллекции, если вы хотите увидеть точный шаблон.
Лукас Тшесневски
34

У вас есть блок итератора. Ни один из кодов этого метода никогда не запускается вне вызовов MoveNextвозвращаемого итератора. При вызове метода конечный автомат регистрируется, но создается, и он никогда не выйдет из строя (за исключением крайних случаев, таких как ошибки нехватки памяти, переполнение стека или исключения прерывания потока).

Когда вы действительно попытаетесь повторить последовательность, вы получите исключения.

Вот почему методам LINQ на самом деле нужны два метода, чтобы иметь желаемую семантику обработки ошибок. У них есть частный метод, который является блоком итератора, а затем метод блока без итератора, который ничего не делает, кроме проверки аргумента (так что это можно сделать с нетерпением, а не откладывать), при этом все остальные функции откладываются.

Итак, это общая картина:

public static IEnumerable<T> Foo<T>(
    this IEnumerable<T> souce, Func<T, bool> anotherArgument)
{
    //note, not an iterator block
    if(anotherArgument == null)
    {
        //TODO make a fuss
    }
    return FooImpl(source, anotherArgument);
}

private static IEnumerable<T> FooImpl<T>(
    IEnumerable<T> souce, Func<T, bool> anotherArgument)
{
    //TODO actual implementation as an iterator block
    yield break;
}
Servy
источник
0

Перечислители, как говорили другие, не оцениваются до тех пор, пока они не начнут перечисляться (т. Е. Вызывается IEnumerable.GetNextметод). Таким образом, это

List<int> indexes = "a.b.c.d.e".AllIndexesOf(null).ToList<int>();

не оценивается, пока вы не начнете перечислять, т.е.

foreach(int index in indexes)
{
    // ArgumentNullException
}
Дженна
источник