Java Puzzler: как я в упор не видел очевидный баг
История поиска бага, которая снова подтверждает, что необходимо покрывать код тестами, не пренебрегать локальными переменными и логированием.
805 открытий831 показов
Недавно готовился к демо-показу новой фичи, прокликивал наше старое GUI-приложение и увидел вот такой код. Прежде чем читать следующие параграфы, я рекомендую остановиться на мгновение и внимательно изучить код. Выделите все проблемы, которые вы видите, как если бы вы проводили ревью:
В нашем приложении метод processEntity, конечно, был сильно сложнее, но я убрал всё, что не имеет отношения к делу. Код прошел ревью у опытного разработчика, да и автора неопытным назвать нельзя. Этот код сложно назвать образцом, но проблема здесь вовсе не в красоте или чистоте.
Этот код — часть приложения, которое уже года три как собираются декоммитить. Это приложение — GUI, написанное на дореволюционной технологии. Там null-значения несут бизнес-ценность, null означает «не отображать ничего». А то, что DTO-отображения используются в бизнес-логике, — так исторически сложилось и улучшать это ресурсов и желания нет.
Проблема, о которой я пишу, — это исключение, но даже когда оно выброшено, ты не всегда понимаешь причину. NPE вылетает на строке:
Самое первое, что я начал проверять — когда valueProcessor может быть null. Через минут 15 я понял, что этого быть не может никогда. И даже после этого я не верил, что что-то ещё в этой строке не так.
Сейчас, когда я знаю проблему, я кажусь себе очень глупым. Как я мог не увидеть такой очевидный баг?! Но когда искал, я его в упор не видел. Что делают плохие программисты, когда код не работает? Правильно, перезапускают его! Но я потерян не полностью, я догадался выделить локальную переменную:
Сделал я это привычным движением, используя горячую клавишу IDE, и запустил код, даже не прочитав, что получилось. Снова NPE, но теперь на другой строке:
Я мог бы поставить брейкпоинт в IDE на выброс любого NPE, но локальной переменной оказалось достаточно. Даже моя любимая IDE мне подсказывала — она создала переменную примитивного типа, чему я не придал значения, мой мозг упростил всё — «ну проверка на null же есть».
Но на деле entity.value1 возвращала null, а тернарный оператор приводил null к примитивному long.
Я поигрался с разными вариациями: объявил переменную Long, поменял местами операнды, гуглил и в итоге почитал JLS. И вот там-то я нашёл очень подробное описание зависимости типов от операндов.
Я перечитал JLS пару лет назад, но не вспомнил о том, что существует такая особенность у тернарников. Плохо это или нет, мне сложно сказать. Я бы так никогда не написал. Вообще стараюсь обходиться примитивными значениями, возможно, поэтому я встретился с такой проблемой впервые. Если бы в этом приложении были тесты или хотя бы кто-то развивал его, то, скорее всего, я бы не встретил эту особенность.
Спасибо, что дочитали, и простите что так долго мусолил этот вопрос. Но я надеюсь, что это поможет лучше запомнить такое поведение. Покрывайте ваш код тестами, не пренебрегайте локальными переменными и логированием, они помогут вам не кормить свой синдром самозванца.
А какие очевидные баги вы долго не могли обнаружить в коде?
805 открытий831 показов