c# - ¿Está bien hacer una devolución desde dentro usando bloque?

code-review

Question

Estoy haciendo una revisión de código, y he encontrado un montón de código con el siguiente formato:

public MyResponse MyMethod(string arg)
{
    using (Tracer myTracer = new Tracer(Constants.TraceLog))
    {
        MyResponse abc = new MyResponse();

        // Some code

        return abc;
    }
}

Cuando ejecuto un análisis de código obtengo una advertencia de CA2000 de Microsoft.

En caso de que el código sea reescrito como:

public MyResponse MyMethod(string arg)
{
   MyResponse abc = new MyResponse();

   using (Tracer myTracer = new Tracer(Constants.TraceLog))
   {
       // Some code
   }
   return abc;
}

¿O no importa?

Editar

La línea en la que está informando la advertencia es:

MyResponse abc = new MyResponse();

MyResponse es un conjunto de datos estándar.

El mensaje de error completo es:

Advertencia 150 CA2000: Microsoft.Reliability: En el método ''xxxxx (Guid, Guid)'', el objeto ''MyResponse'' no está dispuesto a lo largo de todas las rutas de excepción. Llame a System.IDisposable.Dispose en el objeto ''MyResponse'' antes de que todas las referencias a él estén fuera de alcance.

Answer 1

Esta advertencia probablemente esté relacionada con el principio de "punto de salida único". Se trata aquí: http://c2.com/cgi/wiki?SingleFunctionExitPoint

Answer 2

La advertencia probablemente sea sobre MyResponse , que es IDisposable .

¿Por qué aparece la advertencia?

Si se construye un objeto MyResponse , pero el código posterior en el método hace que se MyResponse una excepción, se perderán todas las referencias a este objeto (solo tuvimos una, y no logramos devolverla). Esto significa que ya no se puede invocar a Dispose en el objeto, y dependeremos del finalizador de clase para limpiar cualquier recurso.

¿Importa?

En términos generales, solo importará si:

• El IDisposable encapsula un recurso que podría ser necesario "pronto" por otras partes del programa o por otro proceso
• Se lanza una excepción antes de que el método regrese, para desencadenar el "problema"
• El finalizador no lanza ese recurso lo suficientemente pronto o , por alguna razón, el finalizador nunca se ejecuta pero su aplicación no se desactiva.

Así que no, en realidad no debería importar.

¿Como arreglarlo?

public MyResponse MyMethod(string arg)
{
    MyResponse abc = null;
    try {
        abc = new MyResponse();
        using (Tracer myTracer = new Tracer(Constants.TraceLog))
        {
            // Some code
           return abc;
        }
    }
    catch {
        if (abc != null) {
            abc.Dispose();
        }

        throw;
    }
}

Esto garantiza que si el control sale del método por medio de una excepción, abc es null o se ha eliminado correctamente.

Actualizar

Resulta que al usar esta forma de manejar las cosas, una excepción lanzada explícitamente desde MyMethod será rechazada y el número de línea del primer marco de pila se mutará para apuntar a la throw; declaración.

En la práctica, esto significa que si tiene varias declaraciones de throw dentro de MyResponse , y ellos lanzan el mismo tipo de excepción con el mismo mensaje, no podrá saber qué throw fue el responsable exactamente cuando detectó la excepción.

Este es, en mi opinión, un problema puramente académico, pero lo menciono por completo.

Answer 3

No, no importa.

El finally bloque que se genera implícitamente por la instrucción using para manejar la eliminación se ejecutará sin importar dónde coloque la return .

¿Está seguro de que el CA2000 se relaciona con myTracer y no con abc ? Supongo que la advertencia se está produciendo porque MyResponse implementa IDisposable y no está desechando abc antes de regresar. (De cualquier manera, su reescritura sugerida no debería afectar la advertencia).

Answer 4

Realmente no importa Pero, a diferencia de @Aliostad, creo que la versión 2, con el return fuera del bloque de using es mejor estilo.

Mi razonamiento es así:

El bloque de using denota algo que está "abierto" y "cerrado". Este es un tipo de transacción cheep. Cerrar el bloque de using dice que hemos hecho nuestro trabajo y ahora es seguro continuar con otras cosas, como return .

Answer 5

Su reescritura no solucionará esa advertencia de CA2000, porque el problema no es el objeto Tracer , sino el objeto MyResponse .
La documentación establece:

Las siguientes son algunas situaciones en las que la declaración de uso no es suficiente para proteger objetos IDisposibles y puede hacer que se produzca CA2000.
Devolver un objeto desechable requiere que el objeto se construya en un bloque try / finally fuera de un bloque using.

Para corregir la advertencia sin alterar el seguimiento de la pila de sus excepciones (<- clic, es un enlace), use este código:

public MyResponse MyMethod(string arg)
{
   MyResponse tmpResponse = null;
   MyResponse response = null;
   try
   {
       tmpResponse = new MyResponse();

       using (Tracer myTracer = new Tracer(Constants.TraceLog))
       {
           // Some code
       }

       response = tmpResponse;
       tmpResponse = null;
    }
    finally
    {
        if(tmpResponse != null)
            tmpResponse .Dispose();
    }
    return response;
}

¿Por qué? Por favor vea el ejemplo en la documentación enlazada.