Недавно работал на одним большим проектом написанным на 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.
