понедельник, сентября 03, 2007

Об опасности дефолтных значений

Программисты любят дефолтные значения. Они, собственно их и выдумали. Для программиста слово дефолт ласкает слух, у всех других оно вызывает самые неприятные ассоциации (дефолт рубля, большой дефолт 1998 года...). Но неприятности с дефолтами бывают и у программистов.
Жила была одна система. Среди прочего, она обрабатывала некие задачи. Задачи хранились в БД и были у этих задач состояния. В системе эти состояния были представлены перечислением (enum), а в БД, соответственно - числом (int):

public enum TaskState
{
//Ожидает
Waited,
//Назначена и исполняется
Assigned,
//Отклонена
Rejected,
//Завершена
Completed
}

public class Task
{
public TaskState State;
}


Как известно, в C# тип enum может базироваться на любом из базовых перечислимых типов, а по умолчанию для этого используется Int32. Если значения enum не указаны явно, то первому соответствует 0, второму 1 и т.д.
Но вот в один прекрасный день, в системе решили сделать улучшение - добавить возможность приостановки выполнения заданий. В перечисление добавили еще одно значение TaskState.Suspended, а в бизнес логику соответствующие методы:


public enum TaskState
{
//Ожидает
Waited,
//Назначена и исполняется
Assigned,
//Приостановлена
Suspended,
//Отклонена
Rejected,
//Завершена
Completed
}

public class Task
{
public TaskState State;
public void Suspend(){...}
public void Resume(){...}
}


Разрушительный эффект от этих нехитрых действий был потрясающий. В системе неожиданно возникла масса заданий в состоянии Rejected, а тысячи завершенных заданий (Completed) исчезли. Оно и понятно, если раньше состоянию Completed соответствовало число 3, то теперь оно соответствует состоянию Rejected. Самое смешное, что все модульные тесты прошли успешно.
Горячие головы уже спешно писали набросок скрипта для конвертации данных при переходе на новую версию, а всего то надо было явно объявить значения перечисления:

public enum TaskState
{
//Ожидает
Waited = 0,
//Назначена и исполняется
Assigned = 1,
//Приостановлена
Suspended = 4,
//Отклонена
Rejected = 2,
//Завершена
Completed = 3
}

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

Mikhail комментирует...

Можно хранить строковое предстваление enum-ов в базе.

Бороздин Андрей комментирует...

Сергей, привет! Давно и с интересом читаю твой блог и вот теперь хочу вставить свои пять копеек по поводу описанной ошибки и модульных тестов.
То что модульные тесты прошли успешно, это вполне закономерно. Тесты проверяют только то, что написал сам разработчик этих тестов и ничего более (искусственным интеллектом они не обладают :-)). Как правило, разработчики тестируют работоспособность системы, и как я понял, система работоспособной не пострадала. Т.е. существующие тесты показали себя вполне хорошо. Проблема оказалась в некоторой несовместимости новой версии продукта со старыми данными. По моему мнению, избегать таких ошибок помогли бы регулярные инспекции новых решений и обзоры кода. Как правило, обзоры и инспекции эффективнее модульных тестов, а их совместное использование приносит наибольший выигрыш.

Sergey Rozovik комментирует...

to Бороздин Андрей

В тот раз суровые парни - тестировщики спасли незадачливых программеров :)

shabunc комментирует...

ну вообще-то если искать совем лёгких путей, можно было в принципе suspended в конец поставить

public enum TaskState
{
//Ожидает
Waited,
//Назначена и исполняется
Assigned,
//Отклонена
Rejected,
//Завершена
Completed,
//Приостановлена
Suspended,
}

GiNeR комментирует...

Пять баллов ^_^

MatFiz комментирует...

Типичные Magic Numbers.
В БД их не должно быть так же, как и в любом другом месте.

Sergey Rozovik комментирует...

MatFiz
> Типичные Magic Numbers.
Enum всегда позиционировались, как средство для борьбы с Magic Numbers.

> В БД их не должно быть так же, как и в любом другом месте.

И что же у нас должно быть в БД?


to Mikhail
> Можно хранить строковое представление enum-ов в базе.

Можно. Но мне эта идея не очень нравится. По умолчанию SQLParameter приводит enum к int. Для приведения enum к строке постоянно придется проделывать лишние телодвижения. Ну и наконец, int представление гораздо компактнее чем varchar? и это может служить дополнительным аргументом в его пользу.

to shabunc
> можно было в принципе suspended в конец поставить

В точку. Мне бы тогда и писать не про что было... :)