2009-10-21 14 views
5

Supponiamo il seguente codice:Domanda su foreach e delegati

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

Naturalmente, questo è sbagliato, perché tutti i delegati punti per lo stesso articolo. L'alternativa è:

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

In questo caso tutti i delegati indicano il proprio oggetto.

E questo approccio? C'è qualche altra soluzione migliore?

+0

Potrebbe pubblicare il codice completo che compila e mostra la differenza? – empi

+0

Puoi decompilare il primo pezzo di codice usando C# 1.0 e vedrai quale è la differenza –

risposta

11

UPDATE: C'è ampia analisi e commenti su questo tema qui :

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


Questo è un problema estremamente frequente; di solito è segnalato come un bug del compilatore, ma in effetti il ​​compilatore sta facendo la cosa giusta in base alle specifiche. Le funzioni anonime chiudono oltre variabili, non valori e c'è solo una singola variabile di ciclo foreach. Pertanto, ogni lambda si chiude sulla stessa variabile e quindi ottiene il valore corrente di tale variabile.

Questo è sorprendente per quasi tutti, e porta a molta confusione e molte segnalazioni di bug. Siamo considerando cambiando le specifiche e l'implementazione per un'ipotetica versione futura di C# in modo che la variabile di ciclo sia dichiarata logicamente all'interno del costrutto di ciclo, fornendo una variabile "fresca" ogni volta attraverso il ciclo.

Questo sarebbe un cambio di rottura, ma ho il sospetto che il numero di persone che dipendono da questo strano comportamento sia piuttosto basso. Se hai opinioni su questo argomento, sentiti libero di aggiungere commenti ai post del blog menzionati sopra l'aggiornamento sopra. Grazie!

+1

E non è meglio mostrare un avvertimento del compilatore invece di modificare il comportamento? – FerranB

+1

In effetti, un avvertimento del compilatore * potrebbe * essere garantito in questa situazione. Non è chiaro quale sia * migliore *. Considereremo entrambi. –

0

Il problema che si sta affrontando qui è correlato al costrutto del linguaggio come chiusura. Il secondo pezzo di codice risolve il problema.

5

Il secondo blocco di codice è solo l'approccio migliore che è possibile ottenere mantenendo tutte le altre cose uguali.

Tuttavia, potrebbe essere possibile creare una proprietà su Something che richiede Item. A sua volta il codice evento potrebbe accedere a questo Item dal mittente dell'evento o potrebbe essere incluso negli eventarg per l'evento. Quindi eliminando la necessità della chiusura.

Personalmente ho aggiunto "Eliminazione di chiusura non necessaria" come refactoring utile poiché può essere difficile ragionare su di essi.

+0

sì, quello che stavo pensando, ma molto più eloquente :) – Hath

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

sarebbe non solo passare 'i' in nella vostra classe 'qualcosa' e lo uso in args evento di EventX

1

Se ItemCollection è un (generic) List si può usare la sua ForEach -Metodo. Vi darà un ambito fresca per i:

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

Oppure si può usare qualsiasi altro idoneo Linq -Metodo - come 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

Non consiglierei queste come soluzioni ... –