Comments 34
Что значит
Всё зависит от контекста, вот например
Обязан каждый контакт иметь имейл? А если только телефон. Здесь
Поетому проверки на
В Java 8 появились Optional которые позволяют делать эти проверки более красивыми и читаемыми.
Просто пресеките проблему в самом начале: не принимайте nulls
Всё зависит от контекста, вот например
job.getContact().getEmail();
Обязан каждый контакт иметь имейл? А если только телефон. Здесь
null вполне правильное значение, никак не исключительное соответственно не должно никак вызывать исключений.Поетому проверки на
null нужны там где они нужны!В Java 8 появились Optional которые позволяют делать эти проверки более красивыми и читаемыми.
Простите, для кого эта статья? Для новичков? Тогда может по рукам пора дать?
«Урок 1». Вам не нравятся checked exception? Это нормально — примерно половине разработчиков на java они не нравятся. Другой половине — нравятся. И вот что интересно — если их уметь готовить, то они очень даже удобны (речь не идёт о работе со стримами в java8, это единственная область, где они действительно неудобны почти всем). А вот ваш способ решения — давно проверенный антипаттерн. Конкретно эту ошибку нужно обработать здесь и сейчас. А вот если она ломает логику вашего метода — наверх выкинуть надо СВОЁ исключение. И нет, расплываться мыслью по древу не надо — верхний пример прекрасно схлопывается в один try-catch блок
«Урок 2», спасибо КЭП!.. Только что же примеры такие слабые?
«Урок 3», чувак, всё хорошо на своём месте. null — очень полезная фича. В некоторых языках их легко синтаксисом обернуть в Optional, в Java это приходится делать самому. Ну и проверки на null должны быть адекватны — обязательное поле оказалось null? ПАДАЕМ и логируем — это дело надо исправлять. Необязательное поле? — так и хрен же с ним, а вот защититься от падения здесь надо.
PS в общем переработав статью можно свести к одной фразе «при программировании необходимо включать мозги».
«Урок 1». Вам не нравятся checked exception? Это нормально — примерно половине разработчиков на java они не нравятся. Другой половине — нравятся. И вот что интересно — если их уметь готовить, то они очень даже удобны (речь не идёт о работе со стримами в java8, это единственная область, где они действительно неудобны почти всем). А вот ваш способ решения — давно проверенный антипаттерн. Конкретно эту ошибку нужно обработать здесь и сейчас. А вот если она ломает логику вашего метода — наверх выкинуть надо СВОЁ исключение. И нет, расплываться мыслью по древу не надо — верхний пример прекрасно схлопывается в один try-catch блок
«Урок 2», спасибо КЭП!.. Только что же примеры такие слабые?
«Урок 3», чувак, всё хорошо на своём месте. null — очень полезная фича. В некоторых языках их легко синтаксисом обернуть в Optional, в Java это приходится делать самому. Ну и проверки на null должны быть адекватны — обязательное поле оказалось null? ПАДАЕМ и логируем — это дело надо исправлять. Необязательное поле? — так и хрен же с ним, а вот защититься от падения здесь надо.
PS в общем переработав статью можно свести к одной фразе «при программировании необходимо включать мозги».
Урок 1. Не всякое исключение обязательно ловить и пробрасывать собственное — в некоторых случаях «системное» исключение будет не менее информативно, а кол-во кода и классов собственных исключений будет меньше. И еще, нет ничего отвратительнее catch(Exception ex)
Урок 2. Да, исключения бывают штатными и неожиданными, но тут разница только в том, что отсутствие обработки первых — позорно, а вот вторые нужно «почти» не обрабатывать (в гуёвом приложении можно выкинуть окошко «Извините, но что-то плохое случилось», в headless — как минимум, расписать подробно в лог, что мы пытались сделать)
Урок 3. Все сводится к холивару «fail-fast vs defaults», и где лучше исключение, а где — пустые данные — вопрос вечно открытый. А на самом деле, в Java явно недостает конструкции?.. чтобы вместо if(a!=null&&a.getB()!=null&&a.getB().getC()!=null) писать if(a?.getB()?.getC()!=null)
Урок 2. Да, исключения бывают штатными и неожиданными, но тут разница только в том, что отсутствие обработки первых — позорно, а вот вторые нужно «почти» не обрабатывать (в гуёвом приложении можно выкинуть окошко «Извините, но что-то плохое случилось», в headless — как минимум, расписать подробно в лог, что мы пытались сделать)
Урок 3. Все сводится к холивару «fail-fast vs defaults», и где лучше исключение, а где — пустые данные — вопрос вечно открытый. А на самом деле, в Java явно недостает конструкции?.. чтобы вместо if(a!=null&&a.getB()!=null&&a.getB().getC()!=null) писать if(a?.getB()?.getC()!=null)
И еще, нет ничего отвратительнее catch(Exception ex)С этим можно поспорить. Досталось в наследство консольное приложение (c#), которое забирало данные с сервера и рассылало на почту. Запускалось через Task Scheduler ежедневно. И там все ситуации, типа ошибка 404 или ошибка 500, были перехвачены отдельно.
Но в определённый момент оно стало просто виснуть. В настройках расписания было указано, что если вчерашний запуск не закончился, новые экземпляры не запускаются. Оказалось, что появилось новое, не предвиденное автором исключение — ошибка ресолва хоста. Исключение не перехватывалось и программа висело со стандартным .net окном: «я упало, выберите Дебажить/Закрыть».
Просто заменил кучу всяких catch-ей на один
catch (Exception ex), который пишет ошибку в лог, пишет письмо админу и завершает программу и проблема решена.
Имхо, это тоже самое (концептуально), что и работа без catch вообще.
Если не идёт речи о работе в каком-либо контейнере (который, кстати, ловит Exception), то концептуально это может быть полезным кейсом (в большинстве случаев — аналогиным кейсам этих самых контейнеров). Вот Error ловить — совсем нельзя…
Без catch вообще исключение может никем быть не поймано вовсе, а если и поймано, то неизвестно как обработано — может быть, тем же выводом окна «я упало».
А можно было просто изменить настройки винды, чтобы такое окно (я упало, выберите Дебажить/Закрыть) не появлялось. Это один флаг в реестре.
А ещё лучше дописать немного кода, преобразовав приложение в сервис, и в его настройках выбрать требуемое поведение при остановке, например, повторный запуск.
Для таких случаев можно использовать catch (Exception ex) на самом верхнем уровне, чтобы пользователю не показывалось 404 и 500, но это очень специфичное исключение. Например, эта же конструкция на нижних уровнях — уродлива.
// И еще, нет ничего отвратительнее catch(Exception ex)
Почему? Поясните, пожалуйста, я вот иногда так делаю.
Почему? Поясните, пожалуйста, я вот иногда так делаю.
Потому что рискуем зажевать неожиданное исключение и создать ложное впечатление правильности работы юнита (класса, слоя) и долго потом искать реальную причину неправильного поведения системы. Более правильный способ — ловить наиболее конкретизированные исключения, с индивидуальной обработкой (и индивидуальным логгированием), за, возможно, исключением случая, когда неожиданное исключение долетает до пользователя.
А почему не предлагается подход, распространённый в 1С? Примерно так:
т.е. здесь идея в том, что код сам по себе не падает, а переходит в ветку «МнеДалиПлохиеДанные-Бибикаю»
public String getCustomerName() {
if ( ! isCustomerCorrect(customer) ) {
logger.log("in getCustomerName() we have" + customer.toString() + " incorrect value.");
/* если метод не гуёвый, то выбросим здесь exception и дело с концом */
GUI.alert("The value of '\customer\' field is incorrect! \n" + customer.toString());
return null;
};
return customer.getName();
}
т.е. здесь идея в том, что код сам по себе не падает, а переходит в ветку «МнеДалиПлохиеДанные-Бибикаю»
Во-первых, не переходит в ветку «МнеДалиПлохиеДанные-Бибикаю», а падает выше с кодом
Во-вторых, приложение получается «бибикающим», вместо работающего (помните программы-шутки из 90х годов, которые заставляли нажимать ОК 100 раз?)
И в-третьих, GUI.alert по-хорошему не должен быть доступен отовсюду: изолированный код — хороший код (см. закон Деметры)
public String getOrderDescription() {
return getOrderDate() + " " + getCustomerName().substring(0,10) + "...";
}
Во-вторых, приложение получается «бибикающим», вместо работающего (помните программы-шутки из 90х годов, которые заставляли нажимать ОК 100 раз?)
И в-третьих, GUI.alert по-хорошему не должен быть доступен отовсюду: изолированный код — хороший код (см. закон Деметры)
Вашей функции дали плохие данные. Вы сообщаете об этом факте — это хорошо.
Но после сообщения вы возвращаете, опять-таки, плохие данные. Ваша функция — её контракт, если хотите — вернуть имя пользователя. null — это не имя пользователя ни в коей мере, вы проталкиваете плохие данные дальше, рискуя:
1. Упасть где-то ещё — проблемы с нахождением первопричины ошибки
2. Испортить пользовательские данные
3. Где-то ещё плодить кучу костылей, героически справляясь со своим кривым дизайном
Но после сообщения вы возвращаете, опять-таки, плохие данные. Ваша функция — её контракт, если хотите — вернуть имя пользователя. null — это не имя пользователя ни в коей мере, вы проталкиваете плохие данные дальше, рискуя:
1. Упасть где-то ещё — проблемы с нахождением первопричины ошибки
2. Испортить пользовательские данные
3. Где-то ещё плодить кучу костылей, героически справляясь со своим кривым дизайном
Урок 3: Проверки на null прячут ошибки и порождают больше проверок на null.
Проверки на null не прячут ошибки. Обертки вида
if (abc != null) {
}
прячут. Тут поможет assert вместо проверки.
Давайте вычистим код:
for (Job job : workOrder.getJobs().getJobs()) {
Сразу возникает вопрос — сфигаль мы, получив какие-то джобс из них получаем какие-то другие джобс?
В длинном коде:
Jobs jobs = workOrder.getJobs(); if (jobs != null) { List jobList = jobs.getJobs();
хотя бы понятно, что мы из какого-то супер-объекта джобс получаем список. А в короткой версии это вообще неочевидно — wtf/min увеличивается.
> Как может выглядеть параноидальное программирование? Вот типичный пример на Java
И дальше идет пример параноидального программирования. Это совсем не то же самое, что и защитное программирование.
«защитное программирование» это не есть «стелить солому везде, где только можно»
И дальше идет пример параноидального программирования. Это совсем не то же самое, что и защитное программирование.
«защитное программирование» это не есть «стелить солому везде, где только можно»
наш код радостно сломается и сообщит нам об этом.что он упал с NPE на 4ой строке. А почему? да хз, то ли наgetContact(), то ли .getEmail(), а может и на getText(). В страшном коде с кучей проверок мы хотя бы сможем узнать, что у нас null.
Урок 1: Не обрабатывайте исключения локально.
Нет. Как раз урок должен быть: старайтесь обрабатывать ошибки локально. Чем ближе код к месту возникновения ошибки, тем больше он знает про контекст и тем вероятнее может компенсировать ошибку (прописать обработку). Иногда это невозможно, тогда уровень выше обязан сделать это. Через уровень — 99% никто ничто уже не обработает. Улавливаете?
Урок 2: Восстановление не всегда хорошо. Вы должны принимать во внимание ошибки, вызванные средой, например проблемами с сетью
Если вылетает SocketException в результате того, что я дёрнул проводок — ваше приложение падает? Серьёзно?
Урок 3: Проверки на null прячут ошибки и порождают больше проверок на null.
Это не урок. Это НИ О ЧЁМ. Про это целую статью накатать можно. Как можно давать такие размашистые советы, вообще не понимаю. Детский сад, ну, ей богу.
тем вероятнее может компенсировать ошибку
опять же вопрос — как именно он это делает. Есть опасность вернуть неверные данные. И опять же, появляется необходимость пробрасывания и обработки исключений выше — иногда это излишне.
Если вылетает SocketException в результате того, что я дёрнул проводок — ваше приложение падает?
Не намного хуже того, если приложение не падает, а назойливо жалуется, ничего не делает или вообще виснет, пока не вдёрнете проводок на место — вот что имелось ввиду, по-моему.
Про это целую статью накатать можно
Так вся статья о том, насколько и в каких случаях приложение должно быть параноидальным — городить ли проверки на каждую функцию, или «пусть падает» до более общей проверки?
опять же вопрос — как именно он это делает. Есть опасность вернуть неверные данные. И опять же, появляется необходимость пробрасывания и обработки исключений выше — иногда это излишне.
Кому какой вопрос? Речь о том, что изначальный посыл — неверен. Абсолютно.
Не намного хуже того, если приложение не падает, а назойливо жалуется, ничего не делает или вообще виснет, пока не вдёрнете проводок на место — вот что имелось ввиду, по-моему.
Плачу кровавыми слезами. Приложение, падающее при выдёргивании провода — ДЕБИЛИЗМ. Просто вдумайтесь в это, неужели непонятно? Во-первых, зачастую приложения не ограничены в своих возможностях наличием связи. Может быть независимый функционал. Второе — возможно кэширование. Третье — на крайний случай — надо сказать, что данных нет, так как нет связи — не надо тупо падать и не надо делать назойливо, надо просто уведомить и всё. Откройте официальный клиент ВК без связи — он падает? Вот и подумайте ещё раз. Запомните: SocketException, IOException и иже с ними НИКОГДА не должны валить приложение.
Так вся статья о том, насколько и в каких случаях приложение должно быть параноидальным — городить ли проверки на каждую функцию, или «пусть падает» до более общей проверки?
В контексте использования null — утверждение в статье это пшик. Просто ни о чём. Хорошее приложение должно быть устойчивым, а средств достижения много. Об этом в статье ноль.
Запомните: SocketException, IOException и иже с ними НИКОГДА не должны валить приложение.
It depends. Если приложение, например, не смогло прочитать необходимый для запуска конфиг, т. к. его нет на диске и нет каких-нибудь sensible defaults с которыми можно запуститься — вполне логично упасть с информированием пользователя об отсутствии этого конфига (но не с голым IOE, который ничего не говорит пользователю, ессно).
Если некто написал такой код
Скрытый текст
URL url = null;
try {
url = new URL(urlAsString);
} catch (MalformedURLException e) {
logger.error("Malformed URL", e);
}
// Open the connection to the server
HttpURLConnection connection = null;
try {
connection = (HttpURLConnection) url.openConnection();
} catch (IOException e) {
logger.error("Could not connect to " + url, e);
}это означает, что он совершенно не умеет программировать, т.к. не смог продумать все (три?) ветви выполнения в первом try. Чтобы такое не писать, нужно просто понимать, как выполняется программа. Дальнейшие советы (в данном случае) следуют из этого понимания.
немного логики
Нет, серьёзно, для кого-то неочевидно, что после первого catch код упадёт во втором try? А если очевидно, зачем так писать?
А чтобы не писать такой код
Скрытый текст
public static Set findWorkers(WorkOrder workOrder) {
Set people = new HashSet();
Jobs jobs = workOrder.getJobs();
if (jobs != null) {
List jobList = jobs.getJobs();
if (jobList != null) {
for (Job job : jobList) {
Contact contact = job.getContact();
if (contact != null) {
Email email = contact.getEmail();
if (email != null) {
people.add(email.getText());
}
}
}
}
}
return people;
}нужно бить по рукам за return null;, ибо нефиг возвращать "невозможное" значение туда, где ожидается объект. Ну или не бить (не всегда это возможно), но утопать в проверках.
И инициализация локальных переменных в null (в предыдущем примере) тоже не есть хорошая практика, потому что компилятор java не скомпилирует использование неинициализированной переменной, а с присваиванием null — скомпилирует, хотя в данном случае переменная фактически не инициализирована как следует.
Обычно эти проблемы уходят при наработке небольшого опыта кодирования. Попытка рассмотреть их в статье — это хорошо, но, на мой взгляд, выводы слишком "узкие" (решения конкретных случаев, а не сути проблем).
По проблеме второго цитированного куска кода — бить по рукам не за null нужно, а за избыточный функционал (и за использование коллекций-не-генериков) и хреновую архитектуру… хотя да, за null вместо коллекций можно и по кривым рукам хлопнуть.
Вот что такое класс Jobs? Очевидно workOrder должен содержать коллекцию <job>. (бьём по рукам за кривую архитектуру)
Почему jobs.getJobs может вернуть null вместо пустой коллекции? (бьём по рукам за некорректное использование null)
Какой на***н getContact в Job?! (бьём по рукам за кривую архитектуру)
Почему Email это не подкласс Contact (вот это может быть и обоснованными… но архитектура-то получается кривой)
Какого чёрта мы в Set<непонятно_чего> запихиваем email.getText()?!
А как должно быть? А должно быть разделение обязанностей:
Есть метод findWorkers? Пусть возвращает РАБОТНИКОВ! Хотим извлечь текстовые(!) значения их емейл адресов? Обработаем полученную коллекцию работников в другом месте!
И код свёлся бы к
private Set<Worker> findWorkers(WorkOrder workOrder) {
Set<Worker> workers = new HashSet<>();
for(Job job: workOrder.getJobs()) {
workers.add(job.getWorker());
}
return workers;
}
private Set<String> getWorkerContacts(Set<Worker> workers) {
return workers.stream()
.map(Worker::getEmail)
.filter(email -> email != null)
.map(Email::getText)
.collect(Collectors.toSet());
}
public Set<String> getWorkerContacts(WorkOrder workOrder) {
Set<Worker> workers = getWorkers(workOrder);
return getWorkersContacts(workers);
}
PS сори, парсер съел отступы, а на использование корректной разметки нехватает кармы.
Вот что такое класс Jobs? Очевидно workOrder должен содержать коллекцию <job>. (бьём по рукам за кривую архитектуру)
Почему jobs.getJobs может вернуть null вместо пустой коллекции? (бьём по рукам за некорректное использование null)
Какой на***н getContact в Job?! (бьём по рукам за кривую архитектуру)
Почему Email это не подкласс Contact (вот это может быть и обоснованными… но архитектура-то получается кривой)
Какого чёрта мы в Set<непонятно_чего> запихиваем email.getText()?!
А как должно быть? А должно быть разделение обязанностей:
Есть метод findWorkers? Пусть возвращает РАБОТНИКОВ! Хотим извлечь текстовые(!) значения их емейл адресов? Обработаем полученную коллекцию работников в другом месте!
И код свёлся бы к
private Set<Worker> findWorkers(WorkOrder workOrder) {
Set<Worker> workers = new HashSet<>();
for(Job job: workOrder.getJobs()) {
workers.add(job.getWorker());
}
return workers;
}
private Set<String> getWorkerContacts(Set<Worker> workers) {
return workers.stream()
.map(Worker::getEmail)
.filter(email -> email != null)
.map(Email::getText)
.collect(Collectors.toSet());
}
public Set<String> getWorkerContacts(WorkOrder workOrder) {
Set<Worker> workers = getWorkers(workOrder);
return getWorkersContacts(workers);
}
PS сори, парсер съел отступы, а на использование корректной разметки нехватает кармы.
Это не deffencive programming, это говнокод обыкновенный. При вызове, например badlyImplementedGetData(null) будет простой тупой NPE из connection = (HttpURLConnection) url.openConnection();.
public String badlyImplementedGetData(String urlAsString) {
// Convert the string URL into a real URL
URL url = null;
try {
url = new URL(urlAsString);
} catch (MalformedURLException e) {
logger.error("Malformed URL", e);
}
// Open the connection to the server
HttpURLConnection connection = null;
try {
connection = (HttpURLConnection) url.openConnection();
} catch (IOException e) {
logger.error("Could not connect to " + url, e);
}
}Deffencive programming предполагает проверку всех веток. Например, проверка urlAsString на null, отсечение ветки url == null (ранним возвратом, использование исключения или монадическими операциями на j.u.Optional<T> — не суть важно).
Жаль, монады вообще и Optional в частности мало кто знает и использует. В народе бытует мнение, что функциональное программирование — игрушка для теоретиков, и несовместимо с императивщиной. Хотя в данном примере монада Either не только не помешает, но и сильно улучшит код: обработку ошибок нельзя будет пропустить, и программе не будет грозить падение из-за необработанного исключения.
К сожалению, на scala Option (думаю, и на java тоже) может давать значительную дополнительную нагрузку на gc, если нужно иметь в памяти десятки или сотни миллионов optional значений.
Если это значение короткоживущее, то оно будет в TLAB, выделение в котором очень дешевое. Т. е. для короткоживущих Option/Either оверхед для gc будет небольшой. HotSpot, кажется, не имеет оптимизации по размещению таких объектов на стеке, к сожалению, а то могло бы быть ещё приятнее.
Есть ещё пример rust'а, где Option для ссылок (которые всегда не null) и Box (объект в куче) не имеет оверхеда, т. к. представляет в памяти просто нулевой указатель для None и ненулевой для Some(...). Во многих других случаях он имеет оверхед размером в 4 байта на тэг как и остальные такие enum'ы.
Sign up to leave a comment.
Offensive programming: параноидальное, наступательное, атакующее или беззащитное программирование