2010-05-20 9 views
7

Me gustaría saber su opinión sobre una cuestión de estilo de codificación que estoy al margen. Me doy cuenta de que probablemente no haya una respuesta definitiva, pero me gustaría ver si hay una fuerte preferencia en una dirección u otra.Una cuestión de estilo/legibilidad con respecto a la declaración de "uso" de C#

Estoy pasando por una solución que agrega using declaraciones en bastantes lugares. A menudo se encontrará con algo así:

{ 
    log = new log(); 
    log.SomeProperty = something; // several of these 
    log.Connection = new OracleConnection("..."); 
    log.InsertData(); // this is where log.Connection will be used 
    ... // do other stuff with log, but connection won't be used again 
} 

donde log.Connection es un OracleConnection, que implementa IDisposable.

El neatnik en mí quiere cambiarlo a:

{ 
    using (OracleConnection connection = new OracleConnection("...")) 
    { 
     log = new log(); 
     log.SomeProperty = something; 
     log.Connection = conn; 
     log.InsertData(); 
     ... 
    } 
} 

Pero el amante de la brevedad y conseguir el puesto de trabajo-hecho-ligeramente más rápido quiere hacer:

{ 
    log = new log(); 
    log.SomeProperty = something; 
    using (log.Connection = new OracleConnection("...")) 
     log.InsertData(); 
    ... 
} 

Para alguna razón me siento un poco sucio al hacer esto. ¿Consideras esto malo o no? Si crees que esto es malo, ¿por qué? Si es bueno, ¿por qué?

EDITAR: Tenga en cuenta que este es solo un ejemplo (algo inventado) de muchos. Por favor, no se fije en el hecho de que esto indica una clase de registrador con una interfaz poco pensada. Esto no es relevante para mi pregunta, y no estoy en libertad de mejorar las clases de todos modos.

Respuesta

2

Estoy de acuerdo que idealmente debería implementar logIDisposable, pero asumamos que no es posible y abordemos la pregunta que realmente hizo el OP.

La segunda manera es mejor, simplemente porque es menos código que logra lo mismo. No hay ninguna ventaja al introducir una variable adicional connection aquí.

También tenga en cuenta que podría hacer otra inicialización fuera del bloque using. No importará aquí, pero puede importar si estás "usando" algún recurso realmente costoso. Es decir:

log = new log(); 
log.SomeProperty = something; // This can be outside the "using" 
using (OracleConnection connection = new OracleConnection("...")) 
{ 
    log.Connection = conn; 
    log.InsertData(); 
    ... 
} 
+1

Gracias por abordar la pregunta real. Tu punto sobre la realización de la inicialización fuera del bloque de uso es interesante, porque la única razón por la que lo moví dentro del bloque fue porque sentí que era mejor mantener juntos todos los códigos relacionados. No había pensado en el gasto potencial (no es un problema aquí, sino un punto válido a tener en cuenta). –

2

Me gustaría escuchar el neatnik en ti. Me gusta su manera mejor personalmente.

+0

-1 ... pero ¿por qué? –

+0

Porque la segunda forma me pareció muy fea y soy muy particular acerca de cómo se ve el código. – Josh

+0

Muy bien, un -1 para la honestidad. –

4

Ambos son horribles. No hagas ninguno de ellos.

Usted está haciendo lo que yo llamo "high maintenance class" aquí. La clase de alto mantenimiento tiene un contrato que dice "Necesito que me des unos recursos, y se te pide que sepas cuándo terminé con ellos y cómo limpiarlos adecuadamente". Este contrato significa que el usuario de la clase tiene que saber cómo se implementa la clase, violando así el principio de encapsulación y abstracción que motivó la creación de una clase en primer lugar.

Puede decirlo por su comentario: aquí es donde se usa la conexión, sé que la conexión no se volverá a utilizar. ¿Como sabes eso? Solo se sabe si ese es el contrato documentado de la clase. Ese no es un buen contrato para imponer al consumidor de una clase.

Algunas maneras de hacer esto mejor:

1) hacen que el registrador desechable. Haga que limpie la conexión cuando esté lista. La desventaja de esto es que el registrador se aferra a la conexión por más tiempo de lo necesario.

2) haga que InsertData tome la conexión como un parámetro. La persona que llama puede seguir siendo responsable de limpiar la conexión porque el registrador no se aferra a ella.

3) hacer una tercera clase "Inserter" que es desechable y toma un registro y una conexión en su constructor. El insertador elimina la conexión cuando está dispuesta; la persona que llama es responsable de eliminar el insertador.

+1

Eso es interesante porque la regla de oro la mayor parte del tiempo que he encontrado es "Si la creas, es tu trabajo deshacerse de ella". Me sorprendería más ver que el registrador dispone de una conexión que puede o no ser compartida por otros registradores. Como ejemplo, ha pasado un tiempo desde que utilicé SqlCommand directamente, pero si recuerdo, no elimina la conexión que le pasa. – Josh

+0

@Josh: si quieres compartir, haz la opción n. ° 2. La conexión simplemente no debería ser un miembro del log. –

+0

@Josh: Compare eso con los lectores de flujo, que sí poseen la eliminación de la secuencia que están leyendo. Lo importante es que el contrato no sea (1) difícil o (2) sorprendente. Me sorprendería descubrir que un maderero se aferraba a una referencia a una colección eliminada; ¿Cómo sé que el registrador ha terminado? –

0

Si log implementa IDisposable, haga la segunda opción, ya que las llaves son explícitas.En algunos casos se pueden utilizar varios using declaraciones:

using (Graphics g = ...) 
using (Pen p = new Pen ...) 
using (Font f = new Font ...) 
{ 



} 

donde puede salirse con sólo con 1 juego de llaves. Esto evita las locas sangrías.

+0

log no implementa IDisposable. Con eso en mente, ¿preferiría la primera opción "ordenada"? –

+0

Luego, sigue las sugerencias de Eric Lippert. Él es más o menos la boca del caballo en esta área. –

+1

Las sugerencias de Eric serían geniales si yo fuera el autor de las clases que usaban la conexión, pero él no sugirió nada que abordara mi pregunta real. –

1

Las dos formas de usar using son totalmente equivalentes. De cualquier manera, terminas con un registro que todavía está en el alcance pero que tiene una conexión eliminada. Es mucho mejor hacer que el registro sea desechable y disponer de su método de eliminación de su conexión, colocando el registro en la declaración de uso, no en la conexión en sí.

+1

De acuerdo, este ejemplo muestra una interfaz deficiente, pero dejemos eso de lado por ahora. Me doy cuenta de que las dos formas son equivalentes, pero ¿tiene alguna preferencia instintiva de alguna manera? –

+0

@Charles: Mi instinto me dice que la razón por la que enfrenta este dilema es una opción de diseño original pobre (que hace que la conexión sea un miembro). Si realmente no puedes deshacer esa elección, todavía no lo haría.Haría algo parecido al # 1, pero pondría el objeto de registro completamente dentro del alcance del bloque de uso para que no pueda tratar de usarlo una vez que se haya eliminado la conexión. –

+1

Sí, la realidad de ser un programador de contratación es que enfrenta este dilema a diario, y la mayoría de las veces no puede corregir el problema subyacente. Este es un hecho que regularmente veo desarrolladores aparentemente experimentados en SO que no logran captar. Su punto sobre mover toda la referencia del objeto que consume la conexión dentro del bloque que lo usa, para evitar la posibilidad de que use la conexión después de ser desechado, es una buena idea. Gracias por tu contribución. –

Cuestiones relacionadas