Java Puzzler: как я в упор не видел очевидный баг

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

			class Entity {
    public final Long value1;
    public final Long value2;
    
    public Entity(Long value1, Long value2) {
        this.value1 = value1;
        this.value2 = value2;
    }
}

interface ValueProcessor {
    void process(Long value);
}

public class EntityProcessor {

    private final ValueProcessor valueProcessor;

    public EntityProcessor(ValueProcessor valueProcessor) {
        this.valueProcessor = valueProcessor;
    }

    public void processEntity(String state, Entity entity) {
        switch (state) {
            case "1":
                valueProcessor.process(entity == null ? 0 : entity.value1);
                break;
            case "2":
                valueProcessor.process(entity == null ? 1000 : entity.value2);
                break;
        }
    }
}
		

В нашем приложении метод processEntity, конечно, был сильно сложнее, но я убрал всё, что не имеет отношения к делу. Код прошел ревью у опытного разработчика, да и автора неопытным назвать нельзя. Этот код сложно назвать образцом, но проблема здесь вовсе не в красоте или чистоте.

Этот код — часть приложения, которое уже года три как собираются декоммитить. Это приложение — GUI, написанное на дореволюционной технологии. Там null-значения несут бизнес-ценность, null означает «не отображать ничего». А то, что DTO-отображения используются в бизнес-логике, — так исторически сложилось и улучшать это ресурсов и желания нет.

Проблема, о которой я пишу, — это исключение, но даже когда оно выброшено, ты не всегда понимаешь причину. NPE вылетает на строке:

			valueProcessor.process(entity == null ? 0 : entity.value1);
		

Самое первое, что я начал проверять — когда valueProcessor может быть null. Через минут 15 я понял, что этого быть не может никогда. И даже после этого я не верил, что что-то ещё в этой строке не так.

Сейчас, когда я знаю проблему, я кажусь себе очень глупым. Как я мог не увидеть такой очевидный баг?! Но когда искал, я его в упор не видел. Что делают плохие программисты, когда код не работает? Правильно, перезапускают его! Но я потерян не полностью, я догадался выделить локальную переменную:

			case "1":
    long value = entity == null ? 0 : entity.value1;
    valueProcessor.doProcessValue(value);
    break;
		

Сделал я это привычным движением, используя горячую клавишу IDE, и запустил код, даже не прочитав, что получилось. Снова NPE, но теперь на другой строке:

			long value = entity == null ? 0 : entity.value1;
		

Я мог бы поставить брейкпоинт в IDE на выброс любого NPE, но локальной переменной оказалось достаточно. Даже моя любимая IDE мне подсказывала — она создала переменную примитивного типа, чему я не придал значения, мой мозг упростил всё — «ну проверка на null же есть».

Но на деле entity.value1 возвращала null, а тернарный оператор приводил null к примитивному long.

Я поигрался с разными вариациями: объявил переменную Long, поменял местами операнды, гуглил и в итоге почитал JLS. И вот там-то я нашёл очень подробное описание зависимости типов от операндов.

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

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

А какие очевидные баги вы долго не могли обнаружить в коде?