Опасность использования значений по умолчанию в обьявлениях

Недавно работал на одним большим проектом написанным на C++, и во время отладки встретился с довольно опасной вещью. Посмотрим на примерах. В начале был некий метод с парой параметров один из которых задаётся по умолчанию в обьявлении:

Class1::SetEditBox(CString const& value, bool isUppercase = true);

И гдето в коде этот метод по разному вызывался:

obj1.SetEditBox(_T("Volume"), false);
 ...
 obj2.SetEditBox(_T("SerialNo"));

Прошло время и обьявление метода немного дополнили:

Class1::SetEditBox(CString const& value, bool isEditable = false, bool isUppercase = true);

В новых местах использование, всё было хорошо, да вот старый код толком никто так и не посмотрел, таким образом после вызова

obj1.SetEditBox(_T("Volume"), false);

isUppercase становился true. Однако, так как обьём обычно задаётся в числах, то без толкого тестирования различных вариантов, такой случай прошёл незамеченным.
Время шло, система росла и становилась сложнее. С каждой сменой команды разработчиков её брались переписывать и оптимизировать и в какой то момент бросали. Метод задействовался в других классах:

Class2::SetListEdit(CString const& value, bool isEditable = false, bool isUppercase = true)
{
    // ...
    obj4.SetEditBox(value, isEditable, isUppercase);
    // ...
}

И снова толком никто не просмотрел все места, где нужно было внести изменения.
Время шло и обьявление метода снова пополнилось новым аргументом:

Class1::SetEditBox(CString const& value, bool isNumeric = true, bool isEditable = false, bool isUppercase = true);

На этот раз уже гораздо больше кода следовало просмотреть и изменить, но как то изменение забылось и прошло незамеченным. И далее код ещё более расширился:

Class3::AddListEdit(CString const& value, bool isUppercase = true)
{
    Class2 obj5;
    obj5.SetListEdit(value, isUppercase);
}

В результате мы имеем ошибку, которую я назвал «Shifting Boolean».
Таким образом вызов Class3::AddListEdit() и передача аргументом только строки, приводит к тому, что значениее isUppercase становится значением для isEditable в Class2::SetListEdit() и затем становится значением для isNumeric в Class1::SetEditBox().

Если бы значений по умолчанию не было прописано, то компилятор и IDE сразу бы подняли тревогу. Однако тут ужеприходится выбирать между экономией на написании кода и прозрачностью этого самого кода.

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

Рефакторинг этого кода занял у меня 6 часов. Зато я избавился от переменных типа bool, заменил их единственным аргументом enum, так как в данном случае комбинации изначальных аргументов были между собой связаны логически. И теперь следующий разработчик сможет спокойно воспользоваться единственным нужным значением, вместо того, что бы смотреть где что установлено по умолчанию и что нужно прописать руками. Да и перебрать один аргумент через switch() гораздо легче, чем продираться через лес if()-ов для проверки всех возможных комбинаций для bool.

Добавить комментарий

Ваш e-mail не будет опубликован. Обязательные поля помечены *