2009-10-21 7 views
5

Supposons que le code suivant:Question sur foreach et les délégués

foreach(Item i on ItemCollection) 
{ 
    Something s = new Something(); 
    s.EventX += delegate { ProcessItem(i); }; 
    SomethingCollection.Add(s); 
} 

Bien sûr, cela est faux parce que tous les points de délégués au même article. L'alternative est:

foreach(Item i on ItemCollection) 
{ 
    Item tmpItem = i; 
    Something s = new Something(); 
    s.EventX += delegate { ProcessItem(tmpItem); }; 
    SomethingCollection.Add(s); 
} 

Dans ce cas, tous les délégués pointent vers leur propre élément.

Qu'en est-il de cette approche? Il y a une autre meilleure solution?

+0

Pourriez-vous poster un code complet qui compile et montre la différence? – empi

+0

Vous pouvez décompiler le premier morceau de code en utilisant C# 1.0 et vous verrez quelle est la différence –

Répondre

11

MISE À JOUR: Il y a une analyse approfondie et des commentaires sur cette question ici :

http://ericlippert.com/2009/11/12/closing-over-the-loop-variable-considered-harmful-part-one/


C'est un problème extrêmement fréquemment signalé. Habituellement, il est signalé comme un bug de compilateur, mais en fait le compilateur fait la bonne chose selon la spécification. Les fonctions anonymes ferment plus de les variables, et non les valeurs, et il n'y a qu'une seule variable de boucle foreach. Par conséquent, chaque lambda se ferme sur la même variable, et obtient donc la valeur actuelle de cette variable.

Ceci est surprenant pour presque tout le monde, et conduit à beaucoup de confusion et de nombreux rapports de bogues. Nous sommes en considérant en changeant la spécification et la mise en œuvre pour une future version hypothétique de C# afin que la variable de boucle soit logiquement déclarée dans la construction en boucle, donnant une variable "fraîche" à chaque fois dans la boucle.

Ce serait un changement de rupture, mais je soupçonne que le nombre de personnes qui dépendent de ce comportement étrange est assez faible. Si vous avez des opinions à ce sujet, n'hésitez pas à ajouter des commentaires aux articles de blog mentionnés dans la mise à jour ci-dessus. Merci!

+1

Et ce n'est pas mieux de afficher un avertissement du compilateur au lieu de changer le comportement? – FerranB

+1

En effet, un avertissement de compilateur * pourrait être justifié dans cette situation. Ce n'est pas clair ce qui est * meilleur *. Nous allons considérer les deux. –

0

Le problème que vous rencontrez ici est lié à une construction de langage telle que fermeture. Le deuxième morceau de code résout le problème.

5

Le deuxième morceau de code est à peu près la meilleure approche que vous pouvez obtenir toutes les autres choses restent les mêmes.

Toutefois, il est possible de créer une propriété sur Something qui prend Item. À son tour, le code d'événement pourrait accéder à cet Item à l'expéditeur de l'événement ou il pourrait être inclus dans les eventargs de l'événement. D'où l'élimination de la nécessité de la fermeture.

Personnellement, j'ai ajouté "Elimination of Unnecessary Closure" comme refactoring utile car il peut être difficile de raisonner sur eux.

+0

ouais ce que je pensais, mais beaucoup plus éloquent :) – Hath

0
foreach(Item i on ItemCollection) 
{ 
    Something s = new Something(i); 
    s.EventX += (sender, eventArgs) => { ProcessItem(eventArgs.Item);}; 
    SomethingCollection.Add(s); 
} 

voulez-vous passer pas seulement « i » dans votre « quelque chose » de classe et l'utiliser dans les args d'événements de EventX

1

Si ItemCollection est un (generic) List vous pouvez utiliser son ForEach -method. Il vous donnera un champ frais par i:

ItemCollection.ForEach(
    i => 
    { 
     Something s = new Something(); 
     s.EventX += delegate { ProcessItem(i); }; 
     SomethingCollection.Add(s); 
    }); 

Ou vous pouvez utiliser tout autre approprié Linq -method - comme Select:

var somethings = ItemCollection.Select(
     i => 
     { 
      Something s = new Something(); 
      s.EventX += delegate { ProcessItem(i); }; 
      return s; 
     }); 
foreach(Something s in somethings) 
    SomethingCollection.Add(s); 
+0

Je ne recommanderais pas ces solutions comme si ... –