Avertissement de gestion pour une énumération multiple possible de IEnumerable

Dans mon code, j'ai besoin d'utiliser IEnumerable<>plusieurs fois ainsi obtenir l'erreur Resharper de "Énumération multiple possible de IEnumerable".

Exemple de code:

public List<object> Foo(IEnumerable<object> objects)
{
    if (objects == null || !objects.Any())
        throw new ArgumentException();

    var firstObject = objects.First();
    var list = DoSomeThing(firstObject);        
    var secondList = DoSomeThingElse(objects);
    list.AddRange(secondList);

    return list;
}

• Je peux changer le objectsparamètre pour être Listpuis éviter l'énumération multiple possible mais je n'obtiens pas l'objet le plus élevé que je puisse gérer.
• Une autre chose que je peux faire est de convertir IEnumerableà Listau début de la méthode:

 public List<object> Foo(IEnumerable<object> objects)
 {
    var objectList = objects.ToList();
    // ...
 }

Mais c'est juste gênant .

Que feriez-vous dans ce scénario?

Le problème avec la prise IEnumerableen paramètre est qu'elle indique aux appelants "je souhaite énumérer ceci". Il ne leur dit pas combien de fois vous souhaitez énumérer.

Je peux changer le paramètre des objets en List, puis éviter l'énumération multiple possible, mais je n'obtiens pas l'objet le plus élevé que je puisse gérer .

L'objectif de prendre l'objet le plus élevé est noble, mais il laisse place à trop d'hypothèses. Voulez-vous vraiment que quelqu'un passe une requête LINQ to SQL à cette méthode, uniquement pour l'énumérer deux fois (obtenir des résultats potentiellement différents à chaque fois?)

La sémantique qui manque ici est qu'un appelant, qui peut-être ne prend pas le temps de lire les détails de la méthode, peut supposer que vous ne répétez qu'une seule fois - donc il vous passe un objet coûteux. La signature de votre méthode n'indique aucune façon.

En changeant la signature de la méthode en IList/ ICollection, vous expliquerez au moins clairement à vos interlocuteurs quelles sont vos attentes et ils pourront éviter des erreurs coûteuses.

Sinon, la plupart des développeurs qui examinent la méthode peuvent supposer que vous ne répétez qu'une seule fois. Si prendre un IEnumerableest si important, vous devriez envisager de le faire .ToList()au début de la méthode.

C'est dommage .NET n'a pas d'interface qui est IEnumerable + Count + Indexer, sans méthodes Add / Remove etc., ce qui est ce que je soupçonne de résoudre ce problème.

Si vos données vont toujours être reproductibles, ne vous en faites peut-être pas. Cependant, vous pouvez également le dérouler - cela est particulièrement utile si les données entrantes peuvent être volumineuses (par exemple, lecture à partir du disque / réseau):

if(objects == null) throw new ArgumentException();
using(var iter = objects.GetEnumerator()) {
    if(!iter.MoveNext()) throw new ArgumentException();

    var firstObject = iter.Current;
    var list = DoSomeThing(firstObject);  

    while(iter.MoveNext()) {
        list.Add(DoSomeThingElse(iter.Current));
    }
    return list;
}

Remarque J'ai changé un peu la sémantique de DoSomethingElse, mais c'est principalement pour montrer l'utilisation non déroulée. Vous pouvez reconditionner l'itérateur, par exemple. Vous pouvez également en faire un bloc itérateur, ce qui pourrait être bien; alors il n'y a pas list- et vous feriez yield returnles articles tels que vous les obtenez, plutôt que de les ajouter à une liste à retourner.

L'utilisation de IReadOnlyCollection<T>ou IReadOnlyList<T>dans la signature de méthode au lieu de IEnumerable<T>, a l'avantage de rendre explicite le fait que vous devrez peut-être vérifier le nombre avant l'itération ou répéter plusieurs fois pour une autre raison.

Cependant, ils ont un énorme inconvénient qui causera des problèmes si vous essayez de refactoriser votre code pour utiliser des interfaces, par exemple pour le rendre plus testable et plus convivial pour le proxy dynamique. Le point clé est qu'il IList<T>n'hérite pasIReadOnlyList<T> et de la même manière pour les autres collections et leurs interfaces en lecture seule respectives. (En bref, cela est dû au fait que .NET 4.5 voulait conserver la compatibilité ABI avec les versions antérieures. Mais ils n'ont même pas profité de l'occasion pour changer cela dans .NET core. )

Cela signifie que si vous obtenez un IList<T>d'une partie du programme et que vous souhaitez le passer à une autre partie qui attend un IReadOnlyList<T>, vous ne pouvez pas! Vous pouvez cependant passer un IList<T>comme unIEnumerable<T> .

Au final, IEnumerable<T>c'est la seule interface en lecture seule prise en charge par toutes les collections .NET, y compris toutes les interfaces de collection. Toute autre alternative reviendra vous mordre lorsque vous réaliserez que vous vous êtes exclu de certains choix d'architecture. Je pense donc que c'est le type approprié à utiliser dans les signatures de fonction pour exprimer que vous voulez juste une collection en lecture seule.

(Notez que vous pouvez toujours écrire une IReadOnlyList<T> ToReadOnly<T>(this IList<T> list)méthode d'extension qui effectue une conversion simple si le type sous-jacent prend en charge les deux interfaces, mais vous devez l'ajouter manuellement partout lors de la refactorisation, où as IEnumerable<T>est toujours compatible.)

Comme toujours, ce n'est pas un absolu, si vous écrivez du code lourd de base de données où une énumération multiple accidentelle serait un désastre, vous préférerez peut-être un compromis différent.

Si le but est vraiment d'empêcher plusieurs énumérations, la réponse de Marc Gravell est celle à lire, mais en conservant la même sémantique, vous pouvez simplement supprimer le redondant Anyet les Firstappels et aller avec:

public List<object> Foo(IEnumerable<object> objects)
{
    if (objects == null)
        throw new ArgumentNullException("objects");

    var first = objects.FirstOrDefault();

    if (first == null)
        throw new ArgumentException(
            "Empty enumerable not supported.", 
            "objects");

    var list = DoSomeThing(first);  

    var secondList = DoSomeThingElse(objects);

    list.AddRange(secondList);

    return list;
}

Notez que cela suppose que vous n'êtes IEnumerablepas générique ou du moins contraint à être un type de référence.

Je surcharge généralement ma méthode avec IEnumerable et IList dans cette situation.

public static IEnumerable<T> Method<T>( this IList<T> source ){... }

public static IEnumerable<T> Method<T>( this IEnumerable<T> source )
{
    /*input checks on source parameter here*/
    return Method( source.ToList() );
}

Je prends soin d'expliquer dans le résumé des commentaires des méthodes que l'appel à IEnumerable effectuera un .ToList ().

Le programmeur peut choisir de .ToList () à un niveau supérieur si plusieurs opérations sont concaténées, puis appeler la surcharge IList ou laisser ma surcharge IEnumerable s'en occuper.

Si vous avez seulement besoin de vérifier le premier élément, vous pouvez y jeter un œil sans itérer toute la collection:

public List<object> Foo(IEnumerable<object> objects)
{
    object firstObject;
    if (objects == null || !TryPeek(ref objects, out firstObject))
        throw new ArgumentException();

    var list = DoSomeThing(firstObject);
    var secondList = DoSomeThingElse(objects);
    list.AddRange(secondList);

    return list;
}

public static bool TryPeek<T>(ref IEnumerable<T> source, out T first)
{
    if (source == null)
        throw new ArgumentNullException(nameof(source));

    IEnumerator<T> enumerator = source.GetEnumerator();
    if (!enumerator.MoveNext())
    {
        first = default(T);
        source = Enumerable.Empty<T>();
        return false;
    }

    first = enumerator.Current;
    T firstElement = first;
    source = Iterate();
    return true;

    IEnumerable<T> Iterate()
    {
        yield return firstElement;
        using (enumerator)
        {
            while (enumerator.MoveNext())
            {
                yield return enumerator.Current;
            }
        }
    }
}

Tout d'abord, cet avertissement ne signifie pas toujours autant. Je l'ai généralement désactivé après m'être assuré qu'il ne s'agissait pas d'un col de bouteille performant. Cela signifie simplement que le IEnumerableest évalué deux fois, ce qui n'est généralement pas un problème, sauf si le evaluationlui - même prend beaucoup de temps. Même si cela prend beaucoup de temps, dans ce cas, vous n'utilisez qu'un seul élément la première fois.

Dans ce scénario, vous pouvez également exploiter encore plus les puissantes méthodes d'extension linq.

var firstObject = objects.First();
return DoSomeThing(firstObject).Concat(DoSomeThingElse(objects).ToList();

Il est possible d'évaluer la IEnumerableseule fois dans ce cas avec quelques tracas, mais profilez d'abord et voyez si c'est vraiment un problème.