Cerrar una secuencia en la función, ¿una mala idea?

Question

Escribí un código hoy y fue cambiado por otro desarrollador que dijo que era más seguro. No estoy seguro de que esto es correcto ya que no puedo ver la ventaja de lo que se hizo aquí hay algunos ejemplos de código

public byte[] ReadFile(Stream stream) 
{ 
    byte[] result = null; 

    try 
    { 
     // do something with stream 
     result = <result of operation> 
    } 
    finally 
    { 
     stream.Close(); 
    } 

    return result; 
} 

esto fue cambiado a

public byte[] ReadFile(Stream stream) 
{ 
    byte[] result = null; 

    // do something with stream 
    result = <result of operation> 

    return result; 
} 

soy bastante nuevo en C# si la corriente no estar cerrado cuando haya terminado con ella?

Answer 1

En general, la primera versión sería un mal diseño.

Sí, el flujo debe estar cerrado pero preferiblemente con el mismo código (mismo método) que lo abrió. Eso se llama separación de preocupaciones y hace que los errores y la confusión sean menos probables.

Así que, o

• su ReadFile() acepta, por ejemplo, un string fileName y abre y cierra la corriente, o
• lo deja a la persona que llama.

Su método (la segunda versión) se debe utilizar de esta manera:

using (var reader = new FileReader(...)) 
{ 
    // maybe some pre-reading 
    var r = ReadFile(reader); 
    // maybe some post-reading 
} 

Nota que el segundo enfoque también hace que el método más reutilizables.

Answer 2

No hay una respuesta correcta sobre esta pregunta, ya que depende de la arquitectura de su aplicación.

yo diría, sí, porque si en esta función stream no se crea, pero sólo utilizado, por lo que el cierre de la misma dejemos que hasta la persona que llama.

Pero repito, depende de la arquitectura de su aplicación.

Answer 3

Quien abre la puerta debe recordar cerrarla.

Así que es mejor cerrar la secuencia en el método que lo ha abierto.

Answer 4

La revisión del código era correcta: nunca haga tal cosa (o si un código exterior así lo requiere, entonces probablemente el código no se diseñó correctamente). Hay excepciones, pero las cosas como las transmisiones casi nunca, y siempre se deben seguir algunos "patrones" predeterminados).
en la mayoría de los casos utilizan corrientes de este tipo (de 'fuera') ...

using(Stream stream = new ...) 
{ 
    ...call your method 
} 

(o, por ejemplo lector es la eliminación de la misma - pero se recomienda que se hace algo como esto en cualquier caso - equivalente está utilizando un bloque 'finally', pero se reduce a lo mismo)

... básicamente la función de llamada nunca sabría si la eliminó 'adentro' o no - hasta que se cuelgue. Si 'ambas partes' están de acuerdo con algo así, bueno, todavía no es aceptable, pero podría quedar impune.

Answer 5

Normalmente, el creador de la corriente debe ser el que cerrarlo, idealmente mediante la disposición con un bloque using:

using (var myStream = getMeAStream()) { 
    ReadFile(myStream); 

    // If you want to be really sure it is closed: 
    myStream.Close(); 
    // Probably not neccessary though, since all 
    // implementations of Stream should Close upon Disposal 
} 

Answer 6

La corriente ha pasado a esta función por otra cosa, usted no es responsable en la función para cerrarlo, depende del código de llamada abordar ese problema.

La razón es que es posible que deba realizarse otra operación (como restablecer u otra lectura) de forma externa a esta función y, si la cierra, causará una excepción.

Solía ​​refactorizar códigos como este varias veces al día hasta que finalmente alguien me escuchó.

he visto al menos un grave error causado por este tipo de código, así que es una mala idea en general

Answer 7

No es simplemente un problema de C#. Después de la llamada a la función "ReadFile", ¿necesita usar la transmisión para otras operaciones? Si no, puede elegir cerrar la secuencia después de leer el archivo. es mejor cierre arroyos cuando termine de usarlos, pero en general prefiero para cerrarlas en el mismo contexto que los abre, porque es allí el lugar donde se sabe si necesita corriente de otra operación:

Stream s = open_the_stream() 
try { 
    ReadFile(s)... 
} finally { 
    s.Close(); 
} 

En cualquier caso, cierre la transmisión cuando termine de usarlos.