¿Este es un buen estilo de C#?

Question

Considere el siguiente método de firma:

public static bool TryGetPolls(out List<Poll> polls, out string errorMessage) 

Este método realiza lo siguiente:

• accede a la base de datos para generar una lista de objetos de la encuesta.
• devuelve verdadero si fue correcto y errorMessage será una cadena vacía
• devuelve falso si no fue exitoso y errorMessage contendrá un mensaje de excepción.

¿Es este buen estilo?

Actualización: Digamos que hago uso de la firma siguiente método:

public static List<Poll> GetPolls() 

y en ese método, no coge ninguna excepción (así que dependo de la persona que llama para capturar las excepciones). ¿Cómo elimino y cierro todos los objetos que están en el alcance de ese método? Tan pronto como se lanza una excepción, el código que cierra y elimina objetos en el método ya no es alcanzable.

Answer 1

Ese método es tratar de hacer tres cosas diferentes:

1. recuperar y devolver una lista de encuestas
2. devolver un valor booleano que indica el éxito
3. devolverá un mensaje de error

Eso es bastante desordenado desde el punto de vista del diseño.

Un mejor enfoque sería declarar simplemente:

public static List<Poll> GetPolls() 

A continuación, vamos a este método lanzará una Exception si algo va mal.

Answer 2

creo

public static bool TryGetPolls(out List<Poll> polls) 

sería más apropiado. Si el método es un TryGet, entonces mi suposición inicial sería que hay una razón para esperar que falle, y la persona que llama tiene la responsabilidad de determinar qué hacer a continuación. Si la persona que llama no está manejando el error, o quiere información de error, esperaría que llamaran al método correspondiente Get.

Answer 3

Como regla general, diría que no.

La razón por la que digo que no es en realidad no porque está realizando un TryGetX y devuelve un bool con un parámetro out. Creo que es un mal estilo porque también estás devolviendo una cadena de error.

El Try solo debe ignorar un error específico de uso común. Otros problemas pueden arrojar una excepción con el mensaje de excepción apropiado. Recuerde que el objetivo de un método Try como este es evitar la sobrecarga de una excepción arrojada cuando espera que ocurra un tipo particular de falla con más frecuencia.

En su lugar, lo que estás buscando es un par de métodos:

public static bool TryGetPolls(out List<Poll> polls); 
public static List<Poll> GetPolls(); 

De esta manera el usuario puede hacer lo que es apropiado y GetPolls se pueden implementar en términos de TryGetPolls. Supongo que tu static ness tiene sentido en contexto.

Answer 4

considerar volver:

• una colección vacía
• nula

múltiples parámetros a cabo, para mí, es un code smell. El método debe hacer UNA COSA solamente.

la posibilidad de elevar y gastos de mensajes de error con:

throw new Exception("Something bad happened"); 
//OR 
throw new SomethingBadHappenedException(); 

Answer 5

Depende de lo que el mensaje de error es. Por ejemplo, si el procesamiento no puede continuar porque la conexión de la base de datos no está disponible, etc., entonces debe lanzar una excepción como otras personas han mencionado.

Sin embargo, es posible que solo desee devolver información "meta" sobre el intento, en cuyo caso solo necesita una forma de devolver más de una información de una sola llamada a un método. En ese caso, sugiero hacer una clase PollResponse que contenga dos propiedades: List < Poll> Polls, y string ErrorMessage. Luego haga que su método de devolver un objeto PollResponse:

class PollResponse 
{ 
    public List<Poll> Polls { get; } 
    public string MetaInformation { get; } 
} 

Answer 6

Depende de si un error es una ocurrencia común o si nosotros verdaderamente una excepción.

Si los errores son excepcionalmente raros y malos, entonces es posible que desee considerar tener el método simplemente devolver la lista de encuestas y lanzar una excepción si se produce un error.

Si un error es algo que es una parte realmente común de las operaciones normales, como un error que convierte una cadena en un entero en el método int.TryParse, el método que creó sería más apropiado.

Supongo que la primera es probablemente la mejor opción para usted.

Answer 7

Definitivamente no es una forma idiomática de escribir C#, lo que también significa que probablemente tampoco sea un buen estilo.

Cuando tiene un método TryGetPolls significa que quiere los resultados si la operación tiene éxito, y si no es así, no le importa por qué no tiene éxito.

Cuando simplemente tiene un método GetPolls significa que siempre quiere los resultados, y si no tiene éxito, entonces quiere saber por qué en la forma de un Exception.

La mezcla de los dos está en algún punto intermedio, lo que será inusual para la mayoría de las personas. Así que diría que no devuelves el mensaje de error o lanzas un Exception en caso de falla, pero no utilices este extraño enfoque híbrido.

Así que sus firmas de los métodos probablemente debería ser:

IList<Poll> GetPolls(); 

o

bool TryGetPolls(out IList<Poll> polls); 

(Tenga en cuenta que estoy devolviendo un IList<Poll> en lugar de un List<Poll> en ambos casos también, ya que también es bueno practica programar a una abstracción en lugar de una implementación.)

Answer 8

No, desde mi punto de vista, este es un estilo muy malo. Lo escribiría así:

public static List<Poll> GetPolls(); 

Si la llamada falla, ejecute una excepción y coloque el mensaje de error en la excepción. Para eso están las excepciones y su código será mucho más limpio, más legible y más fácil de mantener.

Answer 9

Depende de la frecuencia con la que falle el método. En general, los errores en .Net deben comunicarse con un Exception. El caso donde esa regla no se cumple es cuando la condición de error es frecuente, y el impacto de throw en el rendimiento y la excepción es demasiado alto.

Para el trabajo del tipo de base de datos, creo que Exception es el mejor.

Answer 10

Lo repetiría así.

public static List<Poll> GetPolls() 
{ 
    ... 
} 

Probablemente se debe lanzar una excepción (la errorMessage) si no puede recuperar las urnas, además esto permite el encadenamiento de método que es menos engorroso que se trata de parámetros out.

Si ejecuta FxCop, querrá cambiar List a IList para mantenerlo feliz.

Answer 11

No realmente - Puedo ver una serie de problemas con esto.

En primer lugar, el método suena como lo que normalmente esperaría que tenga éxito; errores (no se puede conectar a la base de datos, no puede acceder a la tabla polls, etc.) serían raros. En este caso, es mucho más razonable usar excepciones para informar errores. El patrón Try... es para casos en los que a menudo espera que la llamada "falle", p. al analizar una cadena a un número entero, es probable que la cadena sea una entrada del usuario que puede no ser válida, por lo que debe tener una forma rápida de manejar esto, por lo tanto, TryParse. Este no es el caso aquí.

En segundo lugar, informa errores como un valor bool que indica presencia o ausencia de error y un mensaje de cadena. Entonces, ¿cómo distinguiría la persona que llama entre varios errores? Ciertamente, no puede coincidir con el texto del mensaje de error, que es un detalle de implementación que está sujeto a cambios y puede ser localizado. Y podría haber un mundo de diferencia entre algo como "No se puede conectar a la base de datos" (¿tal vez simplemente abra el cuadro de diálogo de configuración de conexión de la base de datos y permita que el usuario lo edite?) Y "Conectado a la base de datos, pero dice 'Acceso denegado' ". Su API no ofrece una buena manera de distinguir entre esos.

Resumiendo: use excepciones en lugar de bool + out string para informar mensajes. Una vez que lo haga, puede simplemente usar List<Poll> como valor de retorno, sin necesidad de argumento out. Y, por supuesto, cambie el nombre del método a GetPolls, ya que Try... está reservado para el patrón bool+out.

Answer 12

Creo que está bien. Yo preferiría sin embargo:

enum FailureReasons {} 
public static IEnumerable<Poll> TryGetPolls(out FailureReasons reason) 

Así las cadenas de error no viven en el código de acceso a datos ...

Answer 13

C# Los métodos deben realmente sólo hacer una cosa. Intentas hacer tres cosas con ese método. Haría lo que otros han sugerido y lanzaré una excepción si hay un error. Otra opción sería crear métodos de extensión para su objeto List.

p. Ej. en una clase public static:

public static List<Poll> Fill(this List<Poll> polls) { 
    // code to retrieve polls 
} 

A continuación, llamar a esto, usted haría algo como:

List<Poll> polls = new List<Poll>().Fill(); 
if(polls != null) 
{ 
    // no errors occur 
} 

EDIT: acabo de hacer esto. puede o no necesitar el nuevo operador en List<Poll>().Fill()

Answer 14

Indique sus suposiciones, limitaciones, deseos/metas y razonamiento; tenemos que adivinar y/o leer tu mente para saber cuáles son tus intenciones.

suponiendo que usted quiere que su función para

• crear el objeto de lista de encuestas
• suprimir todas las excepciones
• indican el éxito con un valor lógico
• y proporcionar un mensaje de error opcional en caso de fallo

luego la firma anterior está bien (aunque tragar todas las excepciones posibles no es una buena p ractice).

Como un estilo de codificación general, tiene algunos problemas potenciales, como han mencionado otros.

Answer 15

Las directrices dicen para tratar de evitar ref y fuera parámetros si no están absolutamente necesario, porque hacen que la API más difícil de usar (no más de encadenamiento de métodos, el desarrollador tiene que declarar todas las variables antes llamar al método)

también códigos de error o mensajes de volver no es una buena práctica, lo mejor es utilizar excepciones y gastos de envío para los informes de error de excepción, los errores de lo contrario se convertirán a la fácil de ignorar y hay más trabajo que pasa el error información a su alrededor, mientras que al mismo tiempo se pierde información valiosa como stacktrace o excepciones internas.

Una mejor forma de declarar el método es la siguiente.

public static List<Poll> GetPolls() ... 

y para el error usar informes de gastos de envío

try 
{ 
    var pols = GetPols(); 
    ... 
} catch (DbException ex) { 
    ... // handle exception providing info to the user or logging it. 
} 

Answer 16

También existe este patrón, como se ve en muchas funciones de Win32 excepción.

public static bool GetPolls(out List<Poll> polls) 


if(!PollStuff.GetPolls(out myPolls)) 
    string errorMessage = PollStuff.GetLastError(); 

Pero IMO es horrible. Me gustaría ir a algo basado en excepción a menos que este método tenga que funcionar 65 veces por segundo en un motor de física de juego en 3D o algo así.

Answer 17

¿Extraño algo aquí? La pregunta asker parece querer saber cómo limpiar los recursos si el método falla.

public static IList<Poll> GetPolls() 
{ 
    try 
    { 
    } 
    finally 
    { 
     // check that the connection happened before exception was thrown 
     // dispose if necessary 
     // the exception will still be presented to the caller 
     // and the program has been set back into a stable state 
    } 
} 

En una nota de diseño, me gustaría considerar empujando este método en una clase repositorio para que tenga algún tipo de contexto con el que entender el método. La aplicación completa, presumiblemente, no es responsable de almacenar y obtener Encuestas: esa debería ser la responsabilidad de un almacén de datos.