Проверка ограничений с использованием IDisposable - безумие или гений?

Question

Я наткнулся на шаблон в кодовой базе, над которым я работаю сегодня, который первоначально казался чрезвычайно умным, потом позже довел меня до безумия, и теперь я задаюсь вопросом, есть ли способ спасти умную часть, минимизируя безумие.

У нас есть набор объектов, которые реализуют IContractObject, и класс InvariantChecker, который выглядит следующим образом:

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
    }
}

Это используется для обеспечения относительно безболезненной проверки согласованности состояний во время выполнения. Я этого не писал, но изначально это казалось довольно крутой идеей.

Однако проблема возникает, если Foo.DoWork выдает исключение. Когда генерируется исключение, вполне вероятно, что мы находимся в противоречивом состоянии, что означает, что InvariantChecker также выбрасывает, скрывая исходное исключение. Это может происходить несколько раз, когда исключение распространяется вверх по стеку вызовов, при этом InvariantChecker в каждом кадре скрывает исключение из кадра ниже. Чтобы диагностировать проблему, мне пришлось отключить бросок в InvariantChecker, и только тогда я смог увидеть исходное исключение.

Это очевидно ужасно. Тем не менее, есть ли способ спасти хитрость оригинальной идеи, не получая ужасного поведения, скрывающего исключения ?

Answer 1

Мне не нравится идея перегрузки значения using таким образом.Почему бы не иметь статический метод, который вместо этого принимает тип делегата?Таким образом, вы бы написали:

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

Или даже лучше, просто сделайте это методом расширения:

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

(Обратите внимание, что часть "this" необходима для того, чтобы получитьКомпилятор C # для поиска методов расширения, применимых к this.) Это также позволяет вам использовать «обычный» метод для реализации действия, если хотите, и использовать преобразование группы методов для создания делегата для него.Возможно, вы захотите разрешить ему возвращать значение, если вы иногда захотите вернуться из тела.

Теперь CheckInvariantActions может использовать что-то вроде:

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

Я бы также предложил1014 * должен, вероятно, генерировать исключение напрямую, а не просто возвращать bool - таким образом, исключение может дать информацию о , инвариант которого был нарушен.

Answer 2

Это ужасное злоупотребление шаблоном использования.Шаблон использования предназначен для удаления неуправляемых ресурсов, а не для «умных» трюков, подобных этойЯ предлагаю просто написать прямой код.

Answer 3

Если вы действительно хотите сделать это:

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 4

Вместо этого:

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

Просто сделайте это ( при условии , что вы не вернетесь с do some stuff):

// do some stuff
this.EnforceInvariants();

Если вам нужночтобы вернуться из do some stuff, я думаю, что рефакторинг в порядке:

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

...

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

Это проще, и у вас не будет проблем, которые у вас были раньше.

Вам просто нужен простой метод расширения:

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

Answer 5

То, что нужно, чтобы сделать это красиво, - это чистый способ выяснить, ожидает ли исключение, когда вызывается Dispose. Либо Microsoft должна предоставить в любое время стандартизированные средства для определения того, какое исключение (если таковое имеется) будет отложено при выходе из текущего блока try-finally, либо Microsoft должна поддерживать расширенный интерфейс Dispose (возможно, DisposeEx, который наследовал бы Dispose), который мог бы принять параметр отложенного исключения.

Answer 6

Если ваша текущая проблема состоит в том, чтобы получить оригинальное исключение - перейдите в Debug -> Exceptions и отметьте «throw» для всех исключений CLR.Он сломается, когда возникнет исключение, и в результате вы увидите его первым.Вам также может потребоваться отключить утилиту-> опции-> отладка-> «мой код только», если исключения вызываются из «не вашего кода» с точки зрения VS.

Answer 7

Добавьте к классу InvariantChecker свойство, позволяющее подавить проверку / бросок.

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;
            }
        }
    }
}