Verificando las restricciones usando IDisposable - ¿locura o genio?

Question

Me encontré con un patrón en una base de código en la que estoy trabajando hoy que inicialmente me pareció extremadamente inteligente, y luego me volví loco, y ahora me pregunto si hay una manera de rescatar la parte inteligente mientras se minimiza la locura.

Tenemos un montón de objetos que implementan IContractObject, y una clase InvariantChecker que tiene este aspecto:

internal class InvariantChecker : IDisposable 
{ 
    private IContractObject obj; 

    public InvariantChecker(IContractObject obj) 
    { 
     this.obj = obj; 
    } 

    public void Dispose() 
    { 
     if (!obj.CheckInvariants()) 
     { 
      throw new ContractViolatedException(); 
     } 
    } 
} 

internal class Foo : IContractObject 
{ 
    private int DoWork() 
    { 
     using (new InvariantChecker(this)) 
     { 
      // do some stuff 
     } 
     // when the Dispose() method is called here, we'll throw if the work we 
     // did invalidated our state somehow 
    } 
} 

Esto se utiliza para proporcionar una validación en tiempo de ejecución relativamente indolora de consistencia estado. No escribí esto, pero inicialmente me pareció una idea genial.

Sin embargo, surge el problema si Foo.DoWork arroja una excepción. Cuando se lanza la excepción, es probable que estemos en un estado incoherente, lo que significa que el InvariantCheckertambién arroja, ocultando la excepción original. Esto puede ocurrir varias veces a medida que la excepción se propaga por la pila de llamadas, con un InvariantChecker en cada cuadro que oculta la excepción del marco a continuación. Para diagnosticar el problema, tuve que deshabilitar el lanzamiento en el InvariantChecker, y solo entonces pude ver la excepción original.

Esto es obviamente terrible. Sin embargo, ¿hay alguna manera de rescatar la astucia de la idea original sin tener el comportamiento horrible de excepción?

Answer 1

Agregue una propiedad a la clase InvariantChecker que le permite suprimir el cheque/tiro.

internal class InvariantChecker : IDisposable 
{ 
    private IContractObject obj; 

    public InvariantChecker(IContractObject obj) 
    { 
     this.obj = obj; 
    } 

    public bool Suppress { get; set; } 

    public void Dispose() 
    { 
     if (!this.Suppress) 
     { 
      if (!obj.CheckInvariants()) 
      { 
       throw new ContractViolatedException(); 
      } 
     } 
    } 
} 

internal class Foo : IContractObject 
{ 
    private int DoWork() 
    { 
     using (var checker = new InvariantChecker(this)) 
     { 
      try 
      { 
       // do some stuff 
      } 
      catch 
      { 
       checker.Suppress = true; 
       throw; 
      } 
     } 
    } 
} 

Answer 2

Este es un horrendo abuso del patrón de uso. El patrón de uso es para deshacerse de recursos no administrados, no para trucos "inteligentes" como este. Sugiero simplemente escribir código directo.

Answer 3

No me gusta la idea de sobrecargar el significado de using de esta manera. ¿Por qué no tener un método estático que toma un tipo de delegado en su lugar? Por lo que iba a escribir:

InvariantChecker.Check(this,() => 
{ 
    // do some stuff 
}); 

O incluso mejor, sólo que sea un método de extensión:

this.CheckInvariantActions(() => 
{ 
    // do some stuff 
}); 

(Tenga en cuenta que se necesita el "este" parte con el fin de obtener el compilador de C# para buscar para los métodos de extensión que se aplican al this). Esto también le permite usar un método "normal" para implementar la acción, si lo desea, y usar una conversión de grupo de métodos para crear un delegado para él. También es posible que desee permitir que devuelva un valor si a veces desea regresar del cuerpo.

ahora CheckInvariantActions puede usar algo como:

action(); 
if (!target.CheckInvariants()) 
{ 
    throw new ContractViolatedException(); 
} 

También sugeriría que CheckInvariants probablemente debería emitir la excepción directamente, en lugar de regresar bool - de esa manera la excepción puede dar información sobre cuales invariante era violado

Answer 4

Si realmente quiere hacer esto:

internal class InvariantChecker : IDisposable 
{ 
    private IContractObject obj; 

    public InvariantChecker(IContractObject obj) 
    { 
     this.obj = obj; 
    } 

    public void Dispose() 
    { 
     if (Marshal.GetExceptionCode() != 0xCCCCCCCC && obj.CheckInvariants()) 
     { 
      throw new ContractViolatedException(); 
     } 
    } 
} 

Answer 5

Si problema actual es conseguir excepción original - ir a la depuración -> Excepciones y active "tirado" para todas las excepciones CLR. Se romperá cuando se arroje la excepción y, como resultado, la verá primero. Es posible que también deba desactivar la opción tools-> options-> debug -> "my code only" si se lanzan excepciones desde "no su código" desde el punto de vista de VS.

Answer 6

Lo que se necesita para hacer esto bien es un medio limpio de averiguar si hay una excepción pendiente cuando se invoca a Dispose. O bien Microsoft debe proporcionar un medio estandarizado para descubrir en cualquier momento qué excepción (si hay alguna) estará pendiente cuando se cierre el bloque try-finally actual, o Microsoft debería admitir una interfaz Dispose extendida (tal vez DisposeEx, que heredaría Dispose) que haría aceptar un parámetro de excepción pendiente.

Answer 7

lugar de esto:

using (new InvariantChecker(this)) { 
    // do some stuff 
} 

a hacer esto (suponiendo no regrese de do some stuff):

// do some stuff 
this.EnforceInvariants(); 

Si necesita volver de do some stuff, creo que algunos refactoring está en orden:

DoSomeStuff(); // returns void 
this.EnforceInvariants(); 

... 

var result = DoSomeStuff(); // returns non-void 
this.EnforceInvariants(); 
return result; 

Es más simple y no tendrá los problemas que tenía antes.

Sólo se necesita un simple método de extensión:

public static class InvariantEnforcer { 
    public static void EnforceInvariants(this IContractObject obj) { 
    if (!obj.CheckInvariants()) { 
     throw new ContractViolatedException(); 
    } 
    } 
}