Pourquoi cette méthode d'extension de chaîne ne lève-t-elle pas d'exception?

119

J'ai une méthode d'extension de chaîne C # qui devrait retourner un IEnumerable<int>de tous les index d'une sous-chaîne dans une chaîne. Il fonctionne parfaitement pour son objectif et les résultats attendus sont renvoyés (comme le prouve l'un de mes tests, mais pas celui ci-dessous), mais un autre test unitaire a découvert un problème: il ne peut pas gérer les arguments nuls.

Voici la méthode d'extension que je teste:

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

Voici le test qui a signalé le problème:

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

Lorsque le test s'exécute sur ma méthode d'extension, il échoue, avec le message d'erreur standard que la méthode "n'a pas lancé d'exception".

C'est déroutant: je suis clairement passé nulldans la fonction, mais pour une raison quelconque, la comparaison null == nullrevient false. Par conséquent, aucune exception n'est levée et le code continue.

J'ai confirmé que ce n'était pas un bogue avec le test: lors de l'exécution de la méthode dans mon projet principal avec un appel à Console.WriteLinedans le ifbloc de comparaison nul , rien n'est affiché sur la console et aucune exception n'est interceptée par un catchbloc que j'ajoute. De plus, utiliser string.IsNullOrEmptyau lieu de == nullpose le même problème.

Pourquoi cette comparaison supposée simple échoue-t-elle?

ArtOfCode
la source
5
Avez-vous essayé de parcourir le code? Cela permettra probablement de le résoudre assez rapidement.
Matthew Haugen
1
Que ne passerait-il? (Est-ce qu'il lève une exception; si oui, laquelle et quelle ligne?)
user2864740
@ user2864740 J'ai décrit tout ce qui se passe. Aucune exception, juste un test échoué et une méthode d'exécution.
ArtOfCode
7
Itérateurs ne sont pas exécutées jusqu'à ce qu'ils soient réitérés-over
BlueRaja - Danny Pflughoeft
2
De rien. Celui-ci a également fait la liste des "pires pièges" de Jon: stackoverflow.com/a/241180/88656 . C'est un problème assez courant.
Eric Lippert

Réponses:

158

Vous utilisez yield return. Ce faisant, le compilateur réécrira votre méthode dans une fonction qui renvoie une classe générée qui implémente une machine à états.

En gros, il réécrit les sections locales dans les champs de cette classe et chaque partie de votre algorithme entre les yield returninstructions devient un état. Vous pouvez vérifier avec un décompilateur ce que devient cette méthode après la compilation (assurez-vous de désactiver la décompilation intelligente qui produirait yield return).

Mais l'essentiel est que le code de votre méthode ne sera pas exécuté tant que vous ne commencerez pas l'itération.

La manière habituelle de vérifier les conditions préalables est de diviser votre méthode en deux:

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

Cela fonctionne car la première méthode se comportera exactement comme vous le souhaitez (exécution immédiate) et renverra la machine à états implémentée par la deuxième méthode.

Notez que vous devez également vérifier le strparamètre null, car les méthodes d'extensions peuvent être appelées sur des nullvaleurs, car elles ne sont que du sucre syntaxique.


Si vous êtes curieux de savoir ce que le compilateur fait à votre code, voici votre méthode, décompilée avec dotPeek à l'aide de l' option Afficher le code généré par le compilateur .

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

Ce code C # n'est pas valide, car le compilateur est autorisé à faire des choses que le langage n'autorise pas, mais qui sont légales en IL - par exemple en nommant les variables d'une manière que vous ne pourriez pas éviter les collisions de noms.

Mais comme vous pouvez le voir, le AllIndexesOfseul construit et renvoie un objet, dont le constructeur n'initialise que certains états. GetEnumeratorcopie uniquement l'objet. Le vrai travail est fait lorsque vous commencez à énumérer (en appelant la MoveNextméthode).

Lucas Trzesniewski
la source
9
BTW, j'ai ajouté le point important suivant à la réponse: Notez que vous devez également vérifier le strparamètre null, car les méthodes d'extensions peuvent être appelées sur des nullvaleurs, car elles ne sont que du sucre syntaxique.
Lucas Trzesniewski
2
yield returnest une bonne idée en principe, mais il y a tellement de pièges bizarres. Merci d'avoir mis celui-ci en lumière!
nateirvin
Donc, fondamentalement, une erreur serait lancée si l'énumérateur était exécuté, comme dans un foreach?
MVCDS
1
@MVCDS Exactement. MoveNextest appelé sous le capot par la foreachconstruction. J'ai écrit une explication de ce que foreachfait dans ma réponse expliquant la sémantique de la collection si vous souhaitez voir le modèle exact.
Lucas Trzesniewski
34

Vous avez un bloc d'itérateur. Aucun code de cette méthode n'est jamais exécuté en dehors des appels à MoveNextsur l'itérateur retourné. L'appel de la méthode ne note mais crée la machine à états, et cela n'échouera jamais (en dehors des extrêmes tels que les erreurs de mémoire insuffisante, les débordements de pile ou les exceptions d'abandon de thread).

Lorsque vous essayez réellement d'itérer la séquence, vous obtenez les exceptions.

C'est pourquoi les méthodes LINQ ont en fait besoin de deux méthodes pour avoir la sémantique de gestion des erreurs qu'elles souhaitent. Ils ont une méthode privée qui est un bloc d'itérateur, puis une méthode de bloc non-itérateur qui ne fait que faire la validation des arguments (afin que cela puisse être fait avec empressement, plutôt que d'être différé) tout en reportant toutes les autres fonctionnalités.

Voici donc le schéma général:

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
la source
0

Les énumérateurs, comme les autres l'ont dit, ne sont évalués qu'au moment où ils commencent à être énumérés (c'est-à-dire que la IEnumerable.GetNextméthode est appelée). Ainsi ce

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

n'est pas évalué tant que vous ne commencez pas à énumérer, c.-à-d.

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