ReadOnlyCollection ou IEnumerable pour exposer les collections de membres?

124

Existe-t-il une raison d'exposer une collection interne en tant que ReadOnlyCollection plutôt que IEnumerable si le code appelant n'itère que sur la collection?

class Bar
{
    private ICollection<Foo> foos;

    // Which one is to be preferred?
    public IEnumerable<Foo> Foos { ... }
    public ReadOnlyCollection<Foo> Foos { ... }
}


// Calling code:

foreach (var f in bar.Foos)
    DoSomething(f);

Comme je le vois, IEnumerable est un sous-ensemble de l'interface de ReadOnlyCollection et il ne permet pas à l'utilisateur de modifier la collection. Donc, si l'interface IEnumberable est suffisante, c'est celle à utiliser. Est-ce une bonne façon de raisonner à ce sujet ou est-ce que je manque quelque chose?

Merci / Erik

Erik Öjebo
la source
6
Si vous utilisez .NET 4.5, vous souhaiterez peut-être essayer les nouvelles interfaces de collecte en lecture seule . Vous voudrez toujours envelopper la collection retournée dans un ReadOnlyCollectionsi vous êtes paranoïaque, mais maintenant vous n'êtes pas lié à une implémentation spécifique.
StriplingWarrior

Réponses:

96

Solution plus moderne

À moins que vous n'ayez besoin que la collection interne soit modifiable, vous pouvez utiliser le System.Collections.Immutablepackage, changer votre type de champ pour être une collection immuable, puis l'exposer directement - en supposant qu'elle Fooest immuable, bien sûr.

Réponse mise à jour pour répondre plus directement à la question

Existe-t-il une raison d'exposer une collection interne en tant que ReadOnlyCollection plutôt que IEnumerable si le code appelant n'itère que sur la collection?

Cela dépend de la confiance que vous accordez au code d'appel. Si vous avez un contrôle total sur tout ce qui appellera ce membre et que vous garantissez qu'aucun code ne l'utilisera jamais:

ICollection<Foo> evil = (ICollection<Foo>) bar.Foos;
evil.Add(...);

alors bien sûr, aucun mal ne sera fait si vous retournez simplement la collection directement. J'essaye généralement d'être un peu plus paranoïaque que ça.

De même, comme vous le dites: si vous avez seulement besoin IEnumerable<T> , alors pourquoi vous attacher à quelque chose de plus fort?

Réponse originale

Si vous utilisez .NET 3.5, vous pouvez éviter de faire une copie et éviter le cast simple en utilisant un simple appel à Skip:

public IEnumerable<Foo> Foos {
    get { return foos.Skip(0); }
}

(Il existe de nombreuses autres options pour encapsuler trivialement - la bonne chose à propos Skipde Select / Where est qu'il n'y a pas de délégué à exécuter inutilement pour chaque itération.)

Si vous n'utilisez pas .NET 3.5, vous pouvez écrire un wrapper très simple pour faire la même chose:

public static IEnumerable<T> Wrapper<T>(IEnumerable<T> source)
{
    foreach (T element in source)
    {
        yield return element;
    }
}
Jon Skeet
la source
15
Notez qu'il y a une pénalité de performance pour cela: AFAIK la méthode Enumerable.Count est optimisée pour les collections castées dans IEnumerables, mais pas pour une plage produite par Skip (0)
shojtsy
6
-1 Est-ce juste moi ou est-ce une réponse à une autre question? Il est utile de connaître cette "solution", mais l'op a demandé une comparaison entre le renvoi d'un ReadOnlyCollection et de IEnumerable. Cette réponse suppose déjà que vous souhaitez renvoyer IEnumerable sans aucun raisonnement pour soutenir cette décision.
Zaid Masud
1
La réponse montre la conversion de a ReadOnlyCollectionen an ICollection, puis l'exécution Addréussie. Autant que je sache, cela ne devrait pas être possible. Le evil.Add(...)devrait générer une erreur. dotnetfiddle.net/GII2jW
Shaun Luttin
1
@ShaunLuttin: Oui, exactement - cela jette. Mais dans ma réponse, je ne lance pas un ReadOnlyCollection- je lance un IEnumerable<T>qui n'est pas réellement en lecture seule ... c'est au lieu d'exposer unReadOnlyCollection
Jon Skeet
1
Aha. Votre paranoïa vous conduirait à utiliser un ReadOnlyCollectionau lieu d'un IEnumerable, afin de vous protéger contre les maléfices.
Shaun Luttin
43

Si vous avez seulement besoin d'itérer dans la collection:

foreach (Foo f in bar.Foos)

puis renvoyer IEnumerable suffit.

Si vous avez besoin d'un accès aléatoire aux éléments:

Foo f = bar.Foos[17];

puis enveloppez-le dans ReadOnlyCollection .

Vojislav Stojkovic
la source
@Vojislav Stojkovic, +1. Ce conseil est-il toujours valable aujourd'hui?
w0051977
1
@ w0051977 Cela dépend de votre intention. Utiliser System.Collections.Immutablecomme Jon Skeet le recommande est parfaitement valable lorsque vous exposez quelque chose qui ne changera jamais après avoir obtenu une référence. D'un autre côté, si vous exposez une vue en lecture seule d'une collection mutable, alors ce conseil est toujours valide, même si je l'exposerais probablement comme IReadOnlyCollection(ce qui n'existait pas lorsque j'ai écrit ceci).
Vojislav Stojkovic
@VojislavStojkovic que vous ne pouvez pas utiliser bar.Foos[17]avec IReadOnlyCollection. La seule différence est la Countpropriété supplémentaire . public interface IReadOnlyCollection<out T> : IEnumerable<T>, IEnumerable
Konrad
@Konrad Oui, si vous en avez besoin bar.Foos[17], vous devez l'exposer comme ReadOnlyCollection<Foo>. Si vous voulez connaître la taille et la parcourir, exposez-la sous la forme IReadOnlyCollection<Foo>. Si vous n'avez pas besoin de connaître la taille, utilisez IEnumerable<Foo>. Il existe d'autres solutions et d'autres détails, mais cela dépasse le cadre de la discussion de cette réponse particulière.
Vojislav Stojkovic
31

Si vous faites cela, rien n'empêche vos appelants de renvoyer le IEnumerable vers ICollection, puis de le modifier. ReadOnlyCollection supprime cette possibilité, bien qu'il soit toujours possible d'accéder à la collection inscriptible sous-jacente via la réflexion. Si la collection est petite, un moyen sûr et facile de contourner ce problème consiste à renvoyer une copie à la place.

Stu Mackellar
la source
14
C'est le genre de "design paranoïaque" avec lequel je ne suis pas du tout d'accord. Je pense que c'est une mauvaise pratique de choisir une sémantique inadéquate pour appliquer une politique.
Vojislav Stojkovic le
24
Vous n'êtes pas d'accord ne vous trompe pas Si je conçois une bibliothèque pour une distribution externe, je préfère de loin avoir une interface paranoïaque que de gérer les bogues soulevés par les utilisateurs qui tentent d'utiliser l'API à mauvais escient. Consultez les directives de conception C # sur msdn.microsoft.com/en-us/library/k2604h5s(VS.71).aspx
Stu Mackellar
5
Oui, dans le cas spécifique de la conception d'une bibliothèque pour une distribution externe, il est logique d'avoir une interface paranoïaque pour tout ce que vous exposez. Même ainsi, si la sémantique de la propriété Foos est un accès séquentiel, utilisez ReadOnlyCollection, puis retournez IEnumerable. Pas besoin d'utiliser une mauvaise sémantique;)
Vojislav Stojkovic
9
Vous n'avez pas besoin de faire non plus. Vous pouvez renvoyer un itérateur en lecture seule sans copier ...
Jon Skeet
1
@shojtsy Votre ligne de code ne semble pas fonctionner. as génère un nul . Un casting lève une exception.
Nick Alexeev
3

J'évite d'utiliser ReadOnlyCollection autant que possible, c'est en fait beaucoup plus lent que d'utiliser simplement une liste normale. Voir cet exemple:

List<int> intList = new List<int>();
        //Use a ReadOnlyCollection around the List
        System.Collections.ObjectModel.ReadOnlyCollection<int> mValue = new System.Collections.ObjectModel.ReadOnlyCollection<int>(intList);

        for (int i = 0; i < 100000000; i++)
        {
            intList.Add(i);
        }
        long result = 0;

        //Use normal foreach on the ReadOnlyCollection
        TimeSpan lStart = new TimeSpan(System.DateTime.Now.Ticks);
        foreach (int i in mValue)
            result += i;
        TimeSpan lEnd = new TimeSpan(System.DateTime.Now.Ticks);
        MessageBox.Show("Speed(ms): " + (lEnd.TotalMilliseconds - lStart.TotalMilliseconds).ToString());
        MessageBox.Show("Result: " + result.ToString());

        //use <list>.ForEach
        lStart = new TimeSpan(System.DateTime.Now.Ticks);
        result = 0;
        intList.ForEach(delegate(int i) { result += i; });
        lEnd = new TimeSpan(System.DateTime.Now.Ticks);
        MessageBox.Show("Speed(ms): " + (lEnd.TotalMilliseconds - lStart.TotalMilliseconds).ToString());
        MessageBox.Show("Result: " + result.ToString());
James Madison
la source
15
Optimisation prématurée. D'après mes mesures, l'itération sur une ReadOnlyCollection prend environ 36% de plus qu'une liste régulière, en supposant que vous ne faites rien à l'intérieur de la boucle for. Il y a de fortes chances que vous ne remarquiez jamais cette différence, mais s'il s'agit d'un point chaud dans votre code et qu'il nécessite le maximum de performances, vous pouvez en tirer parti, pourquoi ne pas utiliser un tableau à la place? Ce serait encore plus rapide.
StriplingWarrior
Votre benchmark contient des failles, vous ignorez complètement la prédiction de branche.
Trojaner
0

Parfois, vous voudrez peut-être utiliser une interface, peut-être parce que vous voulez vous moquer de la collection pendant les tests unitaires. Veuillez consulter mon entrée de blog pour ajouter votre propre interface à ReadonlyCollection à l'aide d'un adaptateur.


la source