¿Cuáles son buenas formas de equilibrar las excepciones informativas y el código limpio?

11

Con nuestro SDK público, tendemos a querer dar mensajes muy informativos sobre por qué ocurre una excepción. Por ejemplo:

if (interfaceInstance == null)
{
     string errMsg = string.Format(
          "Construction of Action Argument: {0}, via the empty constructor worked, but type: {1} could not be cast to type {2}.",
          ParameterInfo.Name,
          ParameterInfo.ParameterType,
          typeof(IParameter)
    );

    throw new InvalidOperationException(errMsg);
}

Sin embargo, esto tiende a saturar el flujo del código, ya que tiende a centrarse mucho en los mensajes de error en lugar de en lo que está haciendo el código.

Un colega comenzó a refactorizar algunas de las excepciones lanzando algo como esto:

if (interfaceInstance == null)
    throw EmptyConstructor();

...

private Exception EmptyConstructor()
{
    string errMsg = string.Format(
          "Construction of Action Argument: {0}, via the empty constructor worked, but type: {1} could not be cast to type {2}.",
          ParameterInfo.Name,
          ParameterInfo.ParameterType,
          typeof(IParameter)
    );

    return new InvalidOperationException(errMsg);
}

Lo que hace que la lógica del código sea más fácil de entender, pero agrega muchos métodos adicionales para manejar los errores.

¿Cuáles son otras formas de evitar el problema de "lógica de desorden de mensajes largos de excepción"? Principalmente estoy preguntando acerca de C # / .NET idiomático, pero también es útil cómo lo manejan otros lenguajes.

[Editar]

Sería bueno tener los pros y los contras de cada enfoque también.

FriendlyGuy
fuente
44
En mi humilde opinión, la solución de tu colega es muy buena, y supongo que si realmente obtienes muchos métodos adicionales de ese tipo, puedes reutilizar al menos algunos de ellos. Tener muchos métodos pequeños está bien cuando sus métodos están bien nombrados, siempre que creen bloques de construcción fáciles de entender para su programa; este parece ser el caso aquí.
Doc Brown
@DocBrown - Sí, me gusta la idea (agregada como respuesta a continuación, junto con los pros / contras), pero tanto para intellisense como para la cantidad potencial de métodos, también comienza a parecer desordenada.
FriendlyGuy
1
Pensamiento: No haga que el mensaje sea el portador de todos los detalles de la excepción. Una combinación del uso de la Exception.Datapropiedad, la captura de excepciones "selectivas", la captura del código de llamada y la adición de su propio contexto, junto con la pila de llamadas capturadas, contribuyen con información que debería permitir mensajes mucho menos detallados. Finalmente System.Reflection.MethodBaseparece prometedor para proporcionar detalles para pasar a su método de "construcción de excepción".
radarbob
@radarbob, ¿sugiere que quizás las excepciones son demasiado detalladas? Quizás lo estamos haciendo demasiado parecido al registro.
FriendlyGuy
1
@MackleChan, leí que el paradigma aquí es poner información en un mensaje que intenta decir exactamente lo que sucedió, es necesariamente gramaticalmente correcto y una pretensión de IA: "... a través del constructor vacío funcionó, pero ..." De Verdad? Mi codificador forense interno ve esto como una consecuencia evolutiva del error común de las repeticiones de pérdida de seguimiento de la pila y la falta de conciencia Exception.Data. El énfasis debe ser capturar la telemetría. Refactorizar aquí está bien, pero pierde el problema.
radarbob

Respuestas:

10

¿Por qué no tener clases de excepción especializadas?

if (interfaceInstance == null)
{
    throw new ThisParticularEmptyConstructorException(<maybe a couple parameters>);
}

Eso empuja el formato y los detalles a la excepción en sí, y deja la clase principal despejada.

ptyx
fuente
1
Pros: muy limpio y organizado, proporciona nombres significativos para cada excepción. Contras: potencialmente muchas clases de excepciones adicionales .
FriendlyGuy
Si es importante, podría tener un número limitado de clases de excepción, pasar la clase de dónde se originó la excepción como parámetro y tener un interruptor gigante dentro de clases de excepción más genéricas. Pero eso tiene un ligero olor, no estoy seguro de que vaya allí.
ptyx
que huele muy mal (excepciones estrechamente vinculadas con las clases). Supongo que sería difícil equilibrar los mensajes de excepción personalizables con el número de excepciones.
FriendlyGuy
7

Microsoft parece (al mirar la fuente .NET) a veces usa cadenas de recursos / entorno. Por ejemplo ParseDecimal:

throw new OverflowException(Environment.GetResourceString("Overflow_Decimal"));

Pros:

  • Centralizando mensajes de excepciones, permitiendo la reutilización
  • Mantener el mensaje de excepción (posiblemente que no importa codificar) alejado de la lógica de los métodos
  • El tipo de excepción que se lanza es claro
  • Los mensajes pueden ser localizados

Contras:

  • Si se cambia un mensaje de excepción, todos cambian
  • El mensaje de excepción no está tan fácilmente disponible para el código que arroja la excepción.
  • El mensaje es estático y no contiene información sobre qué valores son incorrectos. Si desea formatearlo, hay más desorden en el código.
FriendlyGuy
fuente
2
Dejó un gran beneficio: localización del texto de excepción
17 de 26
@ 17of26 - Buen punto, lo agregó.
FriendlyGuy
Voté su respuesta, pero los mensajes de error hechos de esta manera son "estáticos"; no puede agregarles modificadores como lo está haciendo el OP en su código. Así que efectivamente ha omitido algunas de sus funciones.
Robert Harvey
@RobertHarvey: lo agregué como una estafa. Eso explica por qué las excepciones integradas nunca le brindan información local. Además, para su información, soy OP (conocía esta solución, pero quería saber si otros tienen mejores soluciones).
FriendlyGuy
66
@ 17of26 como desarrollador, odio las excepciones localizadas con pasión. Tengo que deslocalizarlos cada vez que pueda, por ejemplo. google para soluciones.
Konrad Morawski
2

Para el escenario de SDK público, consideraría usar contratos de código de Microsoft, ya que proporcionan errores informativos, verificaciones estáticas y también puede generar documentación para agregar a documentos XML y archivos de ayuda generados por Sandcastle . Es compatible con todas las versiones pagas de Visual Studio.

Una ventaja adicional es que si sus clientes están usando C #, pueden aprovechar sus ensambles de referencia de contrato de código para detectar posibles problemas incluso antes de ejecutar su código.

La documentación completa de los contratos de código está aquí .

James World
fuente
2

La técnica que utilizo es combinar y externalizar la validación y el lanzamiento a una función de utilidad.

El beneficio más importante es que se reduce a una sola línea en la lógica empresarial .

Apuesto a que no puede hacerlo mejor a menos que pueda reducirlo aún más: eliminar todas las validaciones de argumentos y guardias de estado de objeto de la lógica comercial, manteniendo solo las condiciones operativas excepcionales.

Por supuesto, hay formas de hacerlo: lenguaje fuertemente tipado, diseño "no se permiten objetos no válidos en cualquier momento", diseño por contrato , etc.

Ejemplo:

internal static class ValidationUtil
{
    internal static void ThrowIfRectNullOrInvalid(int imageWidth, int imageHeight, Rect rect)
    {
        if (rect == null)
        {
            throw new ArgumentNullException("rect");
        }
        if (rect.Right > imageWidth || rect.Bottom > imageHeight || MoonPhase.Now == MoonPhase.Invisible)
        {
            throw new ArgumentException(
                message: "This is uselessly informative",
                paramName: "rect");
        }
    }
}

public class Thing
{
    public void DoSomething(Rect rect)
    {
        ValidationUtil.ThrowIfRectNullOrInvalid(_imageWidth, _imageHeight, rect);
        // rest of your code
    }
}
rwong
fuente
1

[nota] Copié esto de la pregunta en una respuesta en caso de que haya comentarios al respecto.

Mueva cada excepción a un método de la clase, tomando cualquier argumento que necesite el formato.

private Exception EmptyConstructor()
{
    string errMsg = string.Format(
          "Construction of Action Argument: {0}, via the empty constructor worked, but type: {1} could not be cast to type {2}.",
          ParameterInfo.Name,
          ParameterInfo.ParameterType,
          typeof(IParameter)
    );

    return new InvalidOperationException(errMsg);
}

Incluya todos los métodos de excepción en la región y colóquelos al final de la clase.

Pros:

  • Mantiene el mensaje fuera de la lógica central del método
  • Permite agregar información lógica a cada mensaje (puede pasar argumentos al método)

Contras:

  • Método de desorden. Potencialmente, podría tener muchos métodos que solo devuelven excepciones y no están realmente relacionados con la lógica empresarial.
  • No se pueden reutilizar mensajes en otras clases
FriendlyGuy
fuente
Creo que los inconvenientes que citó superan con creces las ventajas.
neontapir
@neontapir los contras son fáciles de abordar. Los métodos auxiliares deben recibir IntentionRevealingNames y agruparse en algunos #region #endregion(lo que hace que estén ocultos del IDE de forma predeterminada), y si son aplicables en diferentes clases, colóquelos en una internal static ValidationUtilityclase. Por cierto, nunca te quejes de los nombres de identificadores largos frente a un programador de C #.
rwong
Sin embargo, me quejaría de las regiones. En mi opinión, si desea recurrir a regiones, la clase probablemente tenía demasiadas responsabilidades.
neontapir
0

Si puede salirse con la suya con errores un poco más generales, podría escribir una función de conversión genérica estática pública para usted, que infiere el tipo de fuente:

public static I CastOrThrow<I,T>(T t, string source)
{
    if (t is I)
        return (I)t;

    string errMsg = string.Format(
          "Failed to complete {0}, because type: {1} could not be cast to type {2}.",
          source,
          typeof(T),
          typeof(I)
        );

    throw new InvalidOperationException(errMsg);
}


/// and then:

var interfaceInstance = SdkHelper.CastTo<IParameter>(passedObject, "Action constructor");

Hay variaciones posibles (piense SdkHelper.RequireNotNull()), que solo verifican los requisitos en las entradas y las arrojan si fallan, pero en este ejemplo, combinar el reparto con producir el resultado es autodocumentado y compacto.

Si está en .net 4.5, hay formas de hacer que el compilador inserte el nombre del método / archivo actual como parámetro de método (consulte CallerMemberAttibute ). Pero para un SDK, probablemente no pueda exigir a sus clientes que cambien a 4.5.

jdv-Jan de Vaan
fuente
Se trata más de lanzar excepciones en general (y administrar la información en una excepción frente a cuánto satura el código), no sobre este ejemplo específico de conversión.
FriendlyGuy
0

Lo que nos gusta hacer para los errores de lógica de negocios (no necesariamente errores de argumento, etc.) es tener una enumeración única que defina todos los tipos potenciales de errores:

/// <summary>
/// This enum is used to identify each business rule uniquely.
/// </summary>
public enum BusinessRuleId {

    /// <summary>
    /// Indicates that a valid body weight value of a patient is missing for dose calculation.
    /// </summary>
    [Display(Name = @"DoseCalculation_PatientBodyWeightMissing")]
    PatientBodyWeightMissingForDoseCalculation = 1,

    /// <summary>
    /// Indicates that a valid body height value of a patient is missing for dose calculation.
    /// </summary>
    [Display(Name = @"DoseCalculation_PatientBodyHeightMissing")]
    PatientBodyHeightMissingForDoseCalculation = 2,

    // ...
}

Los [Display(Name = "...")]atributos definen la clave en los archivos de recursos que se utilizarán para traducir los mensajes de error.

Además, este archivo se puede usar como punto de partida para encontrar todas las ocurrencias en las que se genera un cierto tipo de error en su código.

La verificación de las Reglas comerciales se puede delegar a clases especializadas de Validator que generan listas de Reglas comerciales violadas.

Luego usamos un tipo de excepción personalizado para transportar las reglas violadas:

[Serializable]
public class BusinessLogicException : Exception {

    /// <summary>
    /// The Business Rule that was violated.
    /// </summary>
    public BusinessRuleId ViolatedBusinessRule { get; set; }

    /// <summary>
    /// Optional: additional parameters to be used to during generation of the error message.
    /// </summary>
    public string[] MessageParameters { get; set; }

    /// <summary>
    /// This exception indicates that a Business Rule has been violated. 
    /// </summary>
    public BusinessLogicException(BusinessRuleId violatedBusinessRule, params string[] messageParameters) {
        ViolatedBusinessRule = violatedBusinessRule;
        MessageParameters = messageParameters;
    }
}

Las llamadas de servicio backend están envueltas en un código genérico de manejo de errores, que traduce la regla de negocio violada en un mensaje de error legible por el usuario:

public object TryExecuteServiceAction(Action a) {
    try {
        return a();
    }
    catch (BusinessLogicException bex) {
        _logger.Error(GenerateErrorMessage(bex));
    }
}

public string GenerateErrorMessage(BusinessLogicException bex) {
    var translatedError = bex.ViolatedBusinessRule.ToTranslatedString();
    if (bex.MessageParameters != null) {
        translatedError = string.Format(translatedError, bex.MessageParameters);
    }
    return translatedError;
}

Aquí ToTranslatedString()hay un método de extensión para enumque pueda leer las claves de recursos de los [Display]atributos y usar elResourceManager para traducir estas claves. El valor de la clave de recurso respectiva puede contener marcadores de posición para string.Format, que coinciden con los proporcionados MessageParameters. Ejemplo de una entrada en el archivo resx:

<data name="DoseCalculation_PatientBodyWeightMissing" xml:space="preserve">
    <value>The dose can not be calculated because the body weight observation for patient {0} is missing or not up to date.</value>
    <comment>{0} ... Patient name</comment>
</data>

Ejemplo de uso:

throw new BusinessLogicException(BusinessRuleId.PatientBodyWeightMissingForDoseCalculation, patient.Name);

Con este enfoque, puede desacoplar la generación del mensaje de error a partir de la generación del error, sin la necesidad de introducir una nueva clase de excepción para cada nuevo tipo de error. Útil si diferentes frontends deberían mostrar diferentes mensajes, si el mensaje mostrado debería depender del idioma del usuario y / o función, etc.

Georg Patscheider
fuente