¿Por qué este método de extensión de cadena no genera una excepción?

119

Tengo un método de extensión de cadena C # que debería devolver uno IEnumerable<int>de todos los índices de una subcadena dentro de una cadena. Funciona perfectamente para el propósito previsto y se devuelven los resultados esperados (como lo demuestra una de mis pruebas, aunque no la siguiente), pero otra prueba unitaria ha descubierto un problema con ella: no puede manejar argumentos nulos.

Aquí está el método de extensión que estoy probando:

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

Aquí está la prueba que señaló el problema:

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

Cuando la prueba se ejecuta en mi método de extensión, falla, con el mensaje de error estándar de que el método "no arrojó una excepción".

Esto es confuso: claramente he pasado nulla la función, pero por alguna razón la comparación null == nullestá regresando false. Por lo tanto, no se lanza ninguna excepción y el código continúa.

He confirmado que esto no es un error con la prueba: cuando ejecuto el método en mi proyecto principal con una llamada a Console.WriteLineen el ifbloque de comparación nula , no se muestra nada en la consola y ningún catchbloque que agregue no detecta ninguna excepción . Además, usar en string.IsNullOrEmptylugar de == nulltiene el mismo problema.

¿Por qué falla esta comparación supuestamente simple?

ArtOfCode
fuente
5
¿Ha intentado recorrer el código? Eso probablemente lo resolverá bastante rápido.
Matthew Haugen
1
Lo que no sucederá? (¿Lanza una excepción; si es así, cuál y qué línea?)
user2864740
@ user2864740 He descrito todo lo que sucede. Sin excepciones, solo una prueba fallida y un método de ejecución.
ArtOfCode
7
Iteradores no se ejecutan hasta que se repitan ciertos-over
BlueRaja - Danny Pflughoeft
2
De nada. Este también apareció en la lista de los "peores problemas" de Jon: stackoverflow.com/a/241180/88656 . Este es un problema bastante común.
Eric Lippert

Respuestas:

158

Estás usando yield return. Al hacerlo, el compilador reescribirá su método en una función que devuelve una clase generada que implementa una máquina de estado.

En términos generales, reescribe locales en campos de esa clase y cada parte de su algoritmo entre las yield returninstrucciones se convierte en un estado. Puede verificar con un descompilador en qué se convierte este método después de la compilación (asegúrese de desactivar la descompilación inteligente que produciría yield return).

Pero la conclusión es: el código de su método no se ejecutará hasta que comience a iterar.

La forma habitual de comprobar las condiciones previas es dividir su método en dos:

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

Esto funciona porque el primer método se comportará tal como lo espera (ejecución inmediata) y devolverá la máquina de estado implementada por el segundo método.

Tenga en cuenta que también debe verificar el strparámetro null, porque los métodos de extensión se pueden llamar en nullvalores, ya que son simplemente azúcar sintáctico.


Si tiene curiosidad acerca de lo que hace el compilador con su código, aquí está su método, descompilado con dotPeek usando la opción Mostrar código generado por el compilador .

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()
  {
  }
}

Este es un código C # inválido, porque el compilador puede hacer cosas que el lenguaje no permite, pero que son legales en IL, por ejemplo, nombrar las variables de una manera que no podría evitar las colisiones de nombres.

Pero como puede ver, el AllIndexesOfúnico construye y devuelve un objeto, cuyo constructor solo inicializa algún estado. GetEnumeratorsolo copia el objeto. El trabajo real se realiza cuando comienzas a enumerar (llamando al MoveNextmétodo).

Lucas Trzesniewski
fuente
9
Por cierto, agregué el siguiente punto importante a la respuesta: Tenga en cuenta que también debe verificar el strparámetro null, porque los métodos de extensión se pueden llamar en nullvalores, ya que son solo azúcar sintáctico.
Lucas Trzesniewski
2
yield returnes una buena idea en principio, pero tiene muchas trampas extrañas. ¡Gracias por sacar este a la luz!
nateirvin
Entonces, ¿básicamente se produciría un error si se ejecutara el enumarator, como en un foreach?
MVCDS
1
@MVCDS Exactamente. MoveNextes llamado bajo el capó por la foreachconstrucción. Escribí una explicación de lo que foreachhace en mi respuesta explicando la semántica de la colección si desea ver el patrón exacto.
Lucas Trzesniewski
34

Tienes un bloque de iterador. Ninguno de los códigos de ese método se ejecuta fuera de las llamadas a MoveNexten el iterador devuelto. Llamar al método no hace notar, pero crea la máquina de estado, y eso nunca fallará (fuera de los extremos, como errores de falta de memoria, desbordamientos de pila o excepciones de aborto de subprocesos).

Cuando realmente intente iterar la secuencia, obtendrá las excepciones.

Es por eso que los métodos LINQ realmente necesitan dos métodos para tener la semántica de manejo de errores que desean. Tienen un método privado que es un bloque iterador, y luego un método de bloque no iterador que no hace más que validar el argumento (para que se pueda hacer con entusiasmo, en lugar de diferirlo) y al mismo tiempo diferir todas las demás funciones.

Entonces este es el patrón general:

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
fuente
0

Los enumeradores, como han dicho los demás, no se evalúan hasta el momento en que comienzan a ser enumerados (es decir IEnumerable.GetNext, se llama al método). Así que este

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

no se evalúa hasta que comienza a enumerar, es decir

foreach(int index in indexes)
{
    // ArgumentNullException
}
Jenna
fuente