Pull to refresh

Comments 36

Ещё очень популярное место возникновения NPE — при chaining-е методов, типа db.getUser().getFriends().first().getName(), которое возникает через мысли программиста типа «ну тут Null вернуться не должен.»
Описанные в посте NPE тривиальны и их причина находится очень быстро и просто. А вот то, что описали Вы как раз и приводит к ситуации «не могут сделать никаких выводов без отладчика».
Лезешь в базу и сразу всё становится понятно — надо оторвать руки тому, кто написал такой код — ни проверки на наличие собственно юзера, ни проверки на наличие у юзера френдов…
Такое решается использованием подхода «без null». К примеру, если getFriends() возвращает Friends ( т.е. какая-то коллекция Friend ), то всегда вместо null можно вернуть static final Friends EMPTY = new Friends(); т.е. можно все еще вызывать методы, просто коллекция пустая. Другие вызовы подобным подходом обвернуть.
getFriends ещё как-то может вернуть пустой список. В общем случае это даже более ожидаемое поведение (хотя гляньте-ка на метод Request.getCookies()).
И даже для getUser для некоторой бизнес-логики может возвращать не null (например какого-нибудь guest'а). Но только если бизнес-логика рассчитывает увидеть какого-то дефолтного юзера. Если нет — полюбому должен быть признак отсутствия, а это null и NPE в случае такого чейнинга.

А вот метод first по контракту названия и контекста однозначно должен попытаться зарезолвить первый элемент итератора, обнаружить отсутствие и свалиться с IllegalStateException. Согласен, это не NPE, но недалеко уехали — без проверок так делать нельзя. В Java. Для других языков могут быть приняты свои соглашения.
IMHO, chaining необходимо запрещать на уровне code style, и требовать присваивать отдельным переменным.
Кстати, это и код делает более читабельным в 50% случаев, т.к. по коду одного и того же блока часто раскиданы одинаковые chaining-и вида getTable().getCurrent().getValue().
Мне одному кажется, что такой код рефакторить надо? Одна строка == одно действие.
Иначе читабельность же резко падает. И растёт ошибкоёмкость кода.
А ещё лучше сразу всё правильно и красиво писать, чтобы никаких NPE не возникало.
Нормально читается, а проверки все можно встроить внутри методов и в случае если нет друзей или имени (да чего угодно) — бросать соответствующий Exception.

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

PS/ Сам php-шник, но думаю, что тут подходы могут быть едины.
См. коммент выше.
Если в chaining упал NPE, то очень трудно однозначно интерпретировать причину.
Количество причин равно длине chain'а, и в итоге нужно анализировать гораздо больше сценариев.
Не проще ли написать несколько операторов присваивания, и точно знать причину?
В случае именно с NPE я понимаю, что цепочные запросы очень сложны в расшифровке.

Но если предположить, что метод всегда возвращает правильный результат, либо бросает Exception — то цепочные запросы не должны быть настолько плохи.

Таким образом:

1. Если логику и проверки перенести в исходный метод и они написаны корректно (покрыты тестами) — цепочные запросы будут использоваться правильно и легко читаемы.
2. Если метод будет возвращать непроверенный результат — это может вызывать NPE и тут цепочные запросы могут усложнить жизнь.

И вот в первом описанном случае я все еще не вижу проблем.
Отличная статья, жаль не могу плюсануть вас, кармы не хватает. Она мне конечно не сильно помогла, но все же. Как раз сейчас заканчиваю курс Java и помню, как в начале наступал на грабли по NPE. :) Сейчас слава богу появилась привычка чекать все variables на null перед передачи ее, как аргумента. А сколько раз я парился из-за проблемы при сравнении двух стрингов по типу (string1==string2) Я ошибку искал дня два наверное) Потом узнал про .equals() Веселая она Java.
Перед тем, как писать что-то адекватное, нужно прочитать не менее адекватный учебник по языку.
В каждом языке свои тонкости, без знания которых гарантировано напишешь 100-500 багов.
Интересно, что сообщество скажет по теме — кидать NPE или не кидать. Или кидать другое исключение? У нас в офисе как-то целый холивар разгорелся. Вот я в коде компонента системы получил обьект, который на проверку оказался null. Возможности поправить вызывающий код с тем, чтобы там бросить специфичное для компонента исключение нет, например, вызовов хренова гора. Я в таких случаях кидаю NPE и пишу сообщение в лог. Как делаете вы?
Проигнорирую ваших джунов, хочу заметить, что ваша ссылка не очень подходит к моему вопросу. Она говорит, что ты или кидаешь исключение, или пишешь в лог и обрабатываешь некорректное состояние. С этим никто не спорит, сам всегда так делал.

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

Вопрос в том — бросить NPE самому или позволить ему случиться естественным образом? Мы так и не пришли к соглашению
У себя пришли к следующему:

Весь код условно делится на внутренний и внешний. Внутренний — я написал метод, я же его и вызываю. Тут никаких проверок на аргументы нет вообще. Внешний — как раз на границе компонентов, вот тут проверки уже есть. Но, в данном случае бросаю не NPE, а IllegalArgumentException. Имхо, ближе по смыслу получается.
з.ы. а вот «назойливое сообщение в лог» считаю абсолютно бесполезным, туда все равно никто не смотрит.
Тут, наверное, зависит от того, как поставлены дела в проекте. Я очень заботился о нашем саппорте (чем понятнее логи, тем меньше саппорт будет дергать программистов), и поэтому полиси в проекте было делать лог как можно более понятным (к тому же, смотреть больше и некуда на сервер сайде). С логами у нас обычно все было в порядке и любая странная надпись тут же регистрировалась саппортом с просьбой разобраться, что там такое.
С джунами эт я погорячился, да :) Неоднократно встречался с этим антипаттерном и адом в логах.

О бросании NPE холивар древний и каждый похоже выбирает для себя что-то свое.
stackoverflow.com/questions/3881/illegalargumentexception-or-nullpointerexception-for-a-null-parameter

Я сам NPE никогда не кидаю, кидаю IllegalArgument или IllegalState
Пожалуй плюс в случае самостоятельного выбрасывания NPE может быть в том, что можно указать сообщение, точно идентифицирующее проблему, что позволит быстрее с ней справиться. В качестве альтернативы можно выбросить и IllegalArgumentException.
Я обычно бросаю IllegalArgumentException в этом случае.
В java.util.* распространена практика кидания NPE.
ИМХО, вполне нормально, если в JavaDoc описаны ВСЕ возможные случаи получения такого NPE.
Тип исключения должен соответствовать уровню абстракции, на котором оно выкидывается.
Более того, код должен перехватывать и обрабатывать исключения с нижнего уровня, приводя их к должному уровню абстракции. Это верно для любых независимых компонентов.
UFO just landed and posted this here
Ну вот товарищи предложили кидать IllegalArgumentException, что кажется подходящим. Хренова гора вызовов — это придуманное условие невозможности изменения кода вызова. Просто по хорошему, если я встречаю такую ситуацию, я правлю вызов так, чтобы ситуация с нулем обрабатывалась на той стороне и вызова к моему компоненту не приходило, и такая ситуация невозможна.
Плюсы checked exception понимаешь только тогда, когда тебе в production прилетел системный RuntimeException из недр framework'а с совершенно неадекватным сообщением об ошибке. :-)
P.S. К сожалению, довольно частая ситуация во всяких решениях, построенных на SAX-парсерах.
UFO just landed and posted this here
А почему не рассмотрели следующую ситуацию? Это конечно упрощенно, но смысл думаю понятен. Раз статья для новичков, то думаю они с такой ситуацией сталкивались и могли не сразу понять от чего вдруг NPE.

public class Test {
public static void main(String[] args) {
Integer i = null;
// Тут какая-нибудь логика с i и null в итоге остается
test(i);
}

private static void test(int i) {
// Тут работаем с i
}
}

К сожалению тег source отработал не так как я себе представлял.
Я написал про этот случай:
Помните, что если метод принимает параметр int, а вы передаёте Integer null, то анбоксинг случится до вызова метода, поэтому NPE будет указывать на строку с вызовом.

Пример приводить не стал, чтобы не удлинять статью: случай похож на последний пример.
UFO just landed and posted this here
Зато s не может быть ни под каким соусом: в девятой строке уже было обращение к s. Если бы s было null, исключение бы случилось в девятой строке.

В многопоточной среде это утверждение, строго говоря, неверно.
Нет. Локальные переменные или параметры в Java невозможно изменить из другого потока.
Верно только за счет иммутабельности String'а :-)
Почему? Моё высказывание верно всегда. Изменение полей объекта, на который ссылается локальная переменная, не есть изменение локальной переменной.
Sign up to leave a comment.

Articles