среда, 12 ноября 2014 г.

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

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).

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

image

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

IfThrowConversion

Обратите внимание, что компилятор 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!

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

15 комментариев:

  1. Этот комментарий был удален автором.

    ОтветитьУдалить
    Ответы
    1. Максим, о чем идет речь? Что имеется ввиду под pre- и code-тегами? Если о коде, то это же не картинки. Я просто использую схему текущей студии и вставляю as html в Windows Live Writer. У меня тут даже народ интересовался, каким таким форматером я пользуюсь, что код выглядит так классно;)

      Удалить
    2. Да, последний кусок кода - это картинка, но это же для того, чтобы предупреждение моего чудо-плагина видно было.

      Удалить
  2. Вообще, раз уж контракты есть в BCL, разработчики компилятора могли бы сами код прекондишенов (и до EndContractBlock) выносить.

    ОтветитьУдалить
    Ответы
    1. Тут у местных отношение к контрактам очень неоднозначное: кто-то использует, кто-то нет. Единых гайдланов нет. Вот и получается, что кто-то решил поддержать контракты нормально - допилил код, а кому-то они не нравятся, вот их и нет. А команда контрактов - очень маленькая и сама этим заниматься не может. В общем, пичалька:(

      Удалить
  3. Кстати, по поводу кода - вот как он у меня в хроме выглядит https://dl.dropboxusercontent.com/s/961kvx7efpu4wgy/shot_141112_120125.png Тот блок, что вверху - совсем не читабелен.

    ОтветитьУдалить
    Ответы
    1. Код поправил, спасибо. Live Writer очень интересно себя ведет: я могу копировать нормально as html код из студии, но если попробовать его перенести таким же образом с места на место в теле поста, то он его покорежит вот таким образом, как на скриншоте.

      Удалить
    2. Да, тот код, который редактировался после вставки - виден плохо, фон убирается хромом

      Удалить
  4. Отлично, наконец есть инспекции для таких ошибок. Помню ты не раз говорил на конференциях и вебинарах об особенностях декларации предусловий в блоках итераторов и асинхроных методах.

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

    ОтветитьУдалить
    Ответы
    1. Спасибо, опечатку поправил! и да, теперь осталось сделать такие же анализаторы для VS14:)

      Удалить
  5. >> LINQ-а, лень которого превосходит лень любого программиста...
    Очень удачная формулировка :)

    ОтветитьУдалить
  6. У меня вопрос возник: Как связаны контракты и многопоточность? И вообще: Существует ли способ формально описать контракт типа, работающего в многопоточном окружении или наоборот, описать контракт типа не предназначенного для работы в условиях многопоточности? Сергей, Вы знаете что ни будь об этом?

    ОтветитьУдалить
    Ответы
    1. В Eiffel-е есть SCOOP (http://se.ethz.ch/~meyer/publications/concurrency/scoop_laser.pdf).
      Так что, в целом, связь между контрактами и многопоточностью присутствует.

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

      За пределами Eiffel-а я ничего не слышал про скрещивание этих вдух вещей.

      Удалить
  7. Есть секция в Framework Design Guidelines с некоторыми рекомендациями по этому поводу: https://msdn.microsoft.com/en-us/library/ms229007(v=vs.100).aspx

    ОтветитьУдалить