Когда предусловия не являются предусловиями

UPDATE: библиотека Code Contract знает о тонкостях реализации таких возможностей как async/await и блоков итераторов. Поэтому описанные ранее проблемы касаются лишь старых (legacy) предусловий, основанных на if-throw. Если вы зашли сюда первый раз, то просто не обращайте внимания на этот абзац!

Что такое предусловие? Это некоторое утверждение, которое должно быть истинным во время вызова метода, причем за его истинность отвечает вызывающая сторона. Предусловия включают в себя проверку аргументов или внутреннего состояния объекта, а его нарушение проявляется в генерации вызываемым кодом исключений ArgumentException для невалидных аргументов, и InvalidOperationException для вызова метода в невалидном состоянии объекта.

Другими словами, предусловия гарантируют, что вызывающий код находится в нужном состоянии и ему передали все нужные данные для выполнения своей работы:

// class CustomStream
public Task<string> ReadString(int position)
{
    if (position < 0)
        throw new ArgumentOutOfRangeException("position");
    if (CanRead)
        throw new InvalidOperationException("Stream is not readable");
 
    return Task.FromResult("42");
}

Предусловия могут генерировать специализированные исключения или же использовать Contract.Requires, суть от этого не меняется. Важно лишь то, что вызывающий код должен получить исключение, что скажет вызывающему методу, что операция даже не была начата.

Однако в языке C# есть ряд высокоуровневых языковых конструкций, которые плохо работают со старыми предусловиями, основанными на конструкциях if-throw, точнее, их использование ведет к генерации исключений не там и не тогда, где это ожидает читатель кода.

Проверка предусловий в блоке итераторов

Первым примером неочевидного поведения if-throw-предусловий является блок итераторов (Iterator Block):

public static IEnumerable<string> ReadLineByLine(string path)
{
    if (path == null) throw new ArgumentNullException("path");
 
    using (var file = File.OpenText(path))
    {
        yield return file.ReadLine();
    }
}

Проблема с этим методом в том, что исключение бросается не в момент вызова метода ReadLineByLine(null), а в момент первого вызова метода MoveNext, т.е. в момент «потребления» итератора. А поскольку IEnumerable<string> лежит в основе LINQ-а, лень которого превосходит лень любого программиста, то фактическое выполнение блока итератора может происходить очень далеко от места его создания:

var lines = ReadLineByLine(null);
var ints = lines.Where(s => s.Length > 0).Select(int.Parse);
ProcessInts(ints);
// ...
static void ProcessInts(IEnumerable<int> ints)
{
    Console.WriteLine(ints.Count()); // ANE вылетит здесь!
    ReadAllLinesAsync(null);
}

ПРИМЕЧАНИЕ
Подробнее о внутреннем устройстве итераторов можно почитать в цикле статей “Итераторы в C#”.

Проверка предусловий в асинхронных методах

Аналогичная проблема с предусловиями существует и в методах, помеченных ключевым словом async:

// Асинхронная версия метода File.ReadAllBytes
public async static Task<string[]> ReadAllLinesAsync(string path)
{
    if (path == null) throw new ArgumentNullException("path");
 
    var list = new List<string>();
    using (var file = File.OpenText(path))
    {
        var str = await file.ReadLineAsync();
        list.Add(str);
    }
 
    return list.ToArray();
}

Проблема с этим кодом в том, что в случае невалидных аргументов вызывающий код не получит исключение в момент вызова метода. Вместо этого, исключение будет проброшено в момент анализа возвращенной задачи.

var task = ReadAllLinesAsync(null); // Все ок! 
ConsumeTask(task);
// ...

private void ConsumeTask(Task<string[]> task)
{
    string content = string.Join("\r\n", task.Result); //Ooops!! ANE
    Console.WriteLine(content);
}

Почему это важно? Потому что нарушение предусловия – это бага в вызывающем коде, а значит операция даже будет начата. Задача в faulted состоянии (как в этом случае) говорит о проблемах в вызываемом коде: класс начал выполнять свою работу, но не справился с ней по какой-то причине.

Эта проблема не является теоретической, и беспокоит меня не с эстетической, а с практической точки зрения: чем быстрее мы можем обнаружить проблему в коде, тем легче найти ее исходную причину! В данном случае мы можем протащить «сломанную» таску на несколько уровней от места ее генерации и потом два дня гадать, почему же наш сервис не работает. Одной из главных особенностей использования Task-ов является удобство композиции и легкость их передачи из одного места в другое.

ПРИМЕЧАНИЕ
Обращаю внимание, что проблема есть лишь с методами, помеченными ключевым словом async, к любым другим асинхронным методам эта проблема не относится!

Решение проблемы

В обоих случаях проблема с legacy-предусловиями решается одинаково – путем разделения метода проверяющего аргументы, от метода с основной реализацией.

// Убираем ключевое слово async
public static Task<string[]> ReadAllLinesAsync(string path)
{
    // Валидируем аргументы и делегируем работу другому методу
    if (path == null) throw new ArgumentNullException("path");
 

    return DoReadAllLinesAsync(path);
}
 
private async static Task<string[]> DoReadAllLinesAsync(string path)
{
    var list = new List<string>();
    using (var file = File.OpenText(path))
    {
        var str = await file.ReadLineAsync();
        list.Add(str);
    }
 
    return list.ToArray();
}

Вариант с блоком итераторов абсолютно аналогичен.

Кто-то тут может сказать, что исходный метод нарушает принцип единой обязанности (SRP, Single Responsibility Principle), хотя, как по мне, это просто еще два примера дырявых абстракций, когда детали реализации «просачиваются» наружу и нам с вами о них приходится задумываться.

Другим решением этой проблемы, является использование предусловий из библиотеки Code Contracts, вместо простых if-throw.

Библиотека Code Contracts знает об особенностях возможностях языка C#, как async/await или блока итераторов. Поэтому вместо простой замены Contract.Requires на if-throw, компилятор контрактов “переделывает” метод таким образом, чтобы предусловия срабатывали сразу же.

Это значит, что следующий код является полностью корректным:

public static IEnumerable<string> ReadLineByLine(string path)
{
    Contract.Requires(path != null);
 
    using (var file = File.OpenText(path))
    {
        yield return file.ReadLine();
    }
}
 

var lines = ReadLineByLine(null); // Нарушение предусловия вылетит здесь!
var ints = lines.Where(s => s.Length > 0).Select(int.Parse);
ProcessInts(ints);

Анализ подозрительных предусловий

Поскольку подобные проблемы в коде встречаются довольно часто , то я решил добавить подобный анализ в свой плагин к РеШарперу Code Contract Extension (данная возможность появилась в версии 0.9.0).

Так, в обоих случаях, сей чудо плагин подскажет, что с кодом что-то не так:

И предложить решение: сконвертировать legacy-предусловие в Contract.Requires.

Обратите внимание, что компилятор Code Contract корректно обрабатывает все виды контрактов (а их, напомню, несколько: Contract.Requires, if-throw + Contract.EndContractBlock, custom guard-ов + ContractArgumentValidatorAttribute). Так, например, чтобы старое предусловие вело себя корректным образом, достаточно добавить EndContractBlock после if-throw:

// Асинхронная версия метода File.ReadAllBytes
public async static Task<string[]> ReadAllLinesAsync(string path)
{
    // Данная проверка является уже полноценным предусловием
    // И Code Contract Rewriter позаботиться о корректном поведении!
    if (path == null) throw new ArgumentNullException("path");
    Contract.EndContractBlock();
 
    var list = new List<string>();
    using (var file = File.OpenText(path))
    {
        var str = await file.ReadLineAsync();
        list.Add(str);
    }
 
    return list.ToArray();
}

З.Ы. Естественно, всё вышеперечисленное работает исключительно при включенной галочке Perform Contract Checks на вкладке Code Contracts!

Дополнительные ссылки

• Обсуждение в Г+ бага в BCL
• [Г+] А нужно ли ключевое слово async
• Цикл статей об итераторах
• Code Contract Extension sources
• ReSharper Code Contract Extension at nuget