2012-01-26 6 views
6

Actualmente estoy leyendo una colección de elementos de una secuencia. Lo hago de la siguiente manera:rendimiento siempre se llama

public class Parser{ 

private TextReader _reader; //Get set in Constructor 
private IEnumerable<Item> _items;  

public IEnumerable<Item> Items{ 
    get{ 
    //I >>thought<< this would prevent LoadItems() from being called twice. 
    return _items ?? (_items = LoadItems()); 
    } 
} 

public IEnumerable<Item> LoadItems(){ 
    while(_reader.Peek() >= 0){ 
    yield return new Item(_reader.ReadLine()); //Actually it's a little different 
    } 
} 
} 

Digamos que tengo una corriente que contiene dos artículos, y hago lo siguiente:

var textReader = //Load textreader here 
var parser = new Parser(textReader); 
var result1 = parser.Items.Count(); 
var result2 = parser.Items.Count(); 

Ahora result1 es 2, mientras que result2 es uno.

Ahora noté que mi verificación nula es inútil? Parece que cada vez que llamo a esa función, se rinde de todos modos.

¿Puede alguien explicarme por qué es esto? Y cuál sería la mejor solución para esta situación (por favor díganme si lo que estoy haciendo es una porquería completa: P).

+0

yup, se debe producir cada vez, ya que no está asignando variable '_items' en cualquier lugar lo que su siempre nula y cargar elementos se llama cada vez. –

+0

Corrígeme si me equivoco pero no lo estoy asignando aquí '_items = LoadItems()' –

Respuesta

5

Debido LoadItems es un enumerable perezoso (utiliza yield) y desea asignarlo a un campo, significa que cada vez que enumerar _items en realidad está causando el bucle dentro LoadItems() a correr de nuevo, es decir, (Enumerable.Count crea una nueva Enumerator cada vez que causa que el cuerpo LoadItems se ejecute de nuevo). Como no está creando el lector de nuevo cada vez dentro de LoadItems, su cursor se colocará al final de la secuencia, por lo que es probable que no pueda leer más líneas. Sospecho que devuelve null y devuelve su objeto Item. la segunda llamada contiene una cadena null.

Las soluciones a este serían a 'darse cuenta' el resultado de LoadItems llamando Enumerable.ToList que le dará una lista concreta:

return _items ?? (_items = LoadItems().ToList()); 

o buscando el lector de nuevo al principio de la corriente (si es posible) de modo que LoadItems pueda volver a ejecutar cada vez de forma idéntica.

Pero yo recomendaría que simplemente se deshaga de la yield ing ing en este caso y devuelva una lista concreta, ya que hay poco beneficio por lo que está pagando el precio de la complejidad sin ganancia.

+0

Eso es lo que pensé. Pero cuando estás leyendo exhaustivamente un recurso y cedes, devolviendo los artículos. ¿Cómo puede asegurarse de no perder los recursos? –

+0

Puede conservar el resultado del lector realizando las líneas de lectura en una lista concreta, ya sea llamando 'Enumerable.ToList' en el resultante' IEnumerable' o construyendo manualmente 'List <>' (u otra estructura de datos) p.ej 'LoadItems(). ToList()' o 'new List (LoadItems())'. –

+0

Aah gracias! ¡Me equivoqué totalmente con la idea del rendimiento! Gracias por explicarlo :) –

1

Su nombre de variable lo ha desviado. En la actualidad

private IEnumerable<Item> _items; 

eres perezoso-cargar y guardar la iterador, mientras que es probable que desee ser perezoso-cargar y guardar los artículos (como el nombre de la variable indica):

public class Parser{ 

private TextReader _reader; //Get set in Constructor 
private List<Item> _items;  

public IEnumerable<Item> Items{ 
    get{ 
    return _items ?? (_items = LoadItems().ToList()); 
    } 
} 

private IEnumerable<Item> LoadItems(){ 
    while(_reader.Peek() >= 0){ 
    yield return new Item(_reader.ReadLine()); //Actually it's a little different 
    } 
} 
} 
1

Considere que yield se utiliza como mano corta. Su código se convirtió en algo así como:

private class <>ImpossibleNameSoItWontCollide : IEnumerator<Item> 
{ 
    private TextReader _rdr; 
    /* other state-holding fields */ 
    public <>ImpossibleNameSoItWontCollide(TextReader rdr) 
    { 
    _rdr = rdr; 
    } 
    /* Implement MoveNext, Current here */ 
} 
private class <>ImpossibleNameSoItWontCollide2 : IEnumerable<Item> 
{ 
    private TextReader _rdr; 
    /* other state-holding fields */ 
    public <>ImpossibleNameSoItWontCollide2(TextReader rdr) 
    { 
    _rdr = rdr; 
    } 
    public <>ImpossibleNameSoItWontCollide GetEnumerator() 
    { 
    return new <>ImpossibleNameSoItWontCollide(_rdr); 
    } 
    /* etc */ 
} 
public IEnumerable<Item> LoadItems() 
{ 
    return new <>ImpossibleNameSoItWontCollide2(_rdr); 
} 

Por lo tanto LoadItems() es de hecho sólo se llama una vez, pero el objeto que devuelve tiene GetEnumerator() llama en él dos veces.

Y dado que el TextReader ha avanzado, esto le da los resultados incorrectos. Sin embargo, tenga en cuenta que conduce a un menor uso de la memoria que a todos los elementos, por lo que tiene beneficios cuando no desea utilizar el mismo conjunto de elementos dos veces.

Ya que hace quieren, es necesario crear un objeto que los almacena:

return _items = _items ?? _items = LoadItems().ToList(); 
+0

¡Ah, increíble, realmente me gusta esta respuesta porque me has explicado qué hace realmente el azúcar sintáctico del rendimiento! –

+0

De nada. Vale la pena añadir que si coloca un bloque 'using' en' LoadItems' para eliminar 'TextReader' cuando termina, entonces el método' Dispose() 'del enumerador lo eliminaría (pero no la enumeración, por lo tanto, si la enumeración no se enumerara realmente, no se llamaría). –