Pull to refresh

Comments 38

Принудительное наследование от abstract class Optional<T> уменьшит гибкость классов.
Аспекты таких ограничений не налагают, но как вариант существующих решений стоит упомянуть.
Не надо ничего наследовать. Посмотрите пожалуйста код внимательней. У Optional два статических метода для создания объектов: of для не пустого значения и absent для нулевого и получается что-то вроде:
 Optional<BigInteger> value = someService.getAmout();
 showValueToUser(value.or(BigInteger.ZERO));

вместо:
BigInteger value = someService.getAmout();
showValueToUser(value == null ? BigInteger.ZERO : value);

Выгода на первый взгляд не очевидна, но читается первый вариант приятней, что уже неплохо. В скале конечно это все выглядит намного лучше с паттерн матчингом.
И еще про саму тему поста. Аспекты конечно это мощная штука, но по-моему для решения такой проблемы в большинстве проектов достаточно придерживаться простых правил, которыми почему-то пренебрегают, например: стараться делать классы неизменяемыми (item 15) и всегда задавать значение по умолчанию, никогда не возвращать null вместо пустой коллекции (item 43), использовать паттерн special case описанный у Фаулера, которым и является по сути Optional и Collections.empty*(). Естественно это не всегда возможно в силу каких-то причин, но опять же в большинстве случаев таким образом можно устранить подавляющее число ошибок связанных с NPE (и не только!) и сделать код более красивым и читабельным.
Спасибо за статью, особенно за список источников.
Так и представляю себе Java-bean с 10ю полями и все обёрнуты в Option. Это, конечно, не вариант.
По второй ссылке автор не решает проблему, а прячет под парой классов. Пример использования говорит о непонимании монад и монадической свертки.
А давайте поступим по другому – просто договоримся что неинициализированных значений не существует. То есть, например, если делаем класс то все его поля инициализируем сразу. А вот если поле может быть null а может быть не null тогда оборачиваем его в какую-нибудь мегасложную структуру вроде Nullable<T>?
1) В классе помимо простых объектов String, Integer, Boolean и т.д. могут использоваться более сложные классы, например без конструктора без параметров.
2) Неиницилизированный сложный объект может быть вполне нормален с точки зрения логики программы.

Поэтому в таком случае я предпочитаю действовать точечно, в этом случае — с помощью аннотаций.

Вы можете переписать аспект под свои нужды, код проекта свободен для использования и модификаций (упрощённая 2-пунктовая BSD лицензия).
Я тут «переспал с мыслью» на эту тему :)
В будущей версии я создам отдельный аспект NullSafeAspectTotal, который как раз будет и поддерживать такую логику.
Нужный аспект оставляем в проекте, другой переименовываем, чтобы компилятор его не видел.
Есть одна проблема, которую мне пока не понятно как решить красиво:
При используемой сейчас lazy-инициализации объектов, в случае отсутсвия контруктора без параметров, такая инициализация будет производиться при каждом обращении. Как сделать так чтобы такая иницилизация была только один раз для каждого экземпляра?

Дмитpий, разрешите упомянуть Вас в главе «Благодарности» с будущей статье по этой теме?
Небольшая заметка — зачем писать if (s!=null) if (!s.isEmpty()), когда можно обойтись просто if(s!=null && !s.isEmpty())?
JVM гарантирует нам, что если первый операнд для && (s!=null в нашем случае) будет false то второй операнд уже не будет вычисляться. А значит наш второй операнд !s.isEmpty() не вызовет NullPointerException, и всё будет работать корректно.
Вы абсолютно правы: if(s!=null && !s.isEmpty()) является более оптимальным описанием этого выражения.
Но я ставил целью показать частоту и всеобщность использования проверки на null, поэтому намеренно разделил выражение для лучшего восприятия материала статьи.
P.S. И да,
public class SomeBean{
private String strField="";

public void setStrField(String strField){
if (strField!=null){ this.strField=strField; }
}

public String getStrField(){
return strField;
}
}

лучше записать как

public class SomeBean{
private String strField="";

public void setStrField(String strField){
if (strField!=null){ this.strField=strField; } else { strField = ""; }
}

public String getStrField(){
return strField;
}
}

Чтоб в результате вызовов вроде:
someBean.setStrField(«Something»);
someBean.setStrField(null);
System.out.println(someBean.getStrField());
получить в конце пустую строку, а не «Something».
P.S. А еще лучше, пожалуй, сделать просто
public String getStrField(){
return strField!=null? strField: "";
}
А если у вас java-bean автоматически маппится куда-нибудь по reflection через поля (JAXB, Hibernate)?

На самом деле тут всё просто — если у вас поле не может быть null, значит вызов setStrField(null) — некорректен, он нарушает контракт метода. В этом случае должен быть выброшен NPE. Это позволит отловить ошибки «обнуления» полей класса в будующем.

В этом плане конечно аннотации JSR-305 хороши, с помощью них могут быть автоматически выставлены проверки на null в байт-коде.
Можно конечно и так, но:
1) если strField используется внутри класса напрямую, то придется писать дополнительный «безопасный» код
2) не оптимально с точки зрения быстродействия, так идет сравнение на null в геттере, обычно операции чтения происходят чаще записи.
3) такой код нужно писать для всех таких полей, что просто неудобно

Принудительная инициализация и @ NullSafe(getter=false) — самый оптимальный путь с точки зрения быстродействия.
if (strField!=null){ this.strField=strField; } else { strField = ""; }

Я тоже думал над отождествлением null ещё и как пустого значения или 0, вместо нынешнего отбрасывания значения.
Мои соображения на этот счёт: если считать, что присваивание null является ошибкой, значит эта ошибка не должна влиять на целостность данных.
Я реализую предложенный Вами подход в виде аннотаций типа @ NullSafe(getter=false,nullIsValue=true), но это потребует некоторого времени.

Николай, разрешите упомянуть Вас в главе «Благодарности» с будущей статье по этой теме?
Оу, я прямо застеснялся. Не знаю, право, стоит ли — я же ничего особо не сделал — но разрешаю, конечно.
Как и все идеи, эту также нужно применять только там, где специфически считаете необходимым из-за контекста приложения. Часто шаблоны и идеологии применяются просто потому что они существуют где надо и наоборот. Тоже самое кстати я могу сказать и про использование beans (бинов), set/get методы это явно необходимо когда используете различные генераторы кода (как например Struts с JSP, там без set/get не получится), если же пишете какие-нибудь внутренние структуры для передачи данных из одной части кода другой, где промежуточный результат не выходит за рамки этих классов/методов/пакетов, то вполне подходят и простые структуры без set/get методов.
Поддержка Java Beans упомянута только потому, что при аспектной инициализации
@NullSafe private SomeJavaBean bean;
нужен универсальный код инициализации через reflection, а требование наличия у Java Beans public конструктора без параметров как нельзя лучше к этому подходит.
Object obj=field.getType().newInstance();
Поэтому для данного аспекта сгодиться любой класс с конструктором без параметров…

Вторая причина упоминания Java Beans:
1) обеспечивает возможность прозрачного и без-проблемного увеличения функциональности бина за счет абстракции логики в get/set,
2) упомянутая Вами совместимость со средствами, активно использующими reflection и требующих стандартного стиля
именования, и таких средств очень много.
больше всего мне понравилось как решили проблему в Kotlin
Есть одно НО, при использовании сторонних библиотек/фреймворков от nullpointer не удастся гарантированно избавиться т.к. когда используемый не Ваш код, проверку все равно придется делать в Вашем куске…, а если навешаете аспекты, то есть шанс сломать стороннюю внутреннюю логику, т.к., к примеру, теже синглтоны могу быть завязаны в чужом коде на проверку на null…
Многие сторонние библиотеки/фреймворки сами реализуют подобную логику, например реализации JPA.
В текущей реализации аспекты ограничены действием аннотации @ NullSafe, которую, по идее, вводят осознанно.
Реализация аспекта NullSafe по умолчанию статична, то есть может действовать только на исходные (то есть Ваши) тексты. Следовательно текущая реалилизация не затронет сторонние библиотеки, если для этого не предпринять дополнительных мер (см. Динамические аспекты или обобщение аннотаций аспектами).
Если синглтон без @ NullSafe, то логика аспекта работать не будет.
Аннотации из jsr 305 удобно использовать совместно с findbugs. Т.е. findbugs, проанализировав код, покажет проблемные места. К тому же Idea (кажется с 10-й версии) понимает эти аннотации (помимо своих из org.jetbrains.annotations) и подсвечивает проблемные места, что позволяет фиксить ошибки сразу, при редактировании кода, не доходя до этапа компиляции и запуска тестов.

С использованием вашего подхода, как мне показалось, мы прячем ошибку в логике, затрудняя её обнаружение. Возможно, к примеру, было бы лучше в «сеттере» бросать NPE если кто-то пытается туда передать null? Кстати, idea при компиляции может автоматически вставлять assert-ы в код для проверки notnull/nullable значений.

И ещё, не боитесь распухания PermGen из-за большого количества классов, ведь ajc для каждого Around метода порождает класс. Или эту проблему как-то можно решить?
Как я уже заметил в статье findbugs показывает, но не даёт решение сам.
findbugs — очень хороший инcтрумент в других случаях, и ничто не мешает использовать его совместно с NullSafeAspect.

прячем ошибку в логике, затрудняя её обнаружение. Возможно, к примеру, было бы лучше в «сеттере» бросать NPE если кто-то пытается туда передать null


Исключение еще нужно где-то перехватывать, лучше логирование без прерывания выполнения для выяснения проблем в логике.
Глючащая в одном маленьком месте программа гораздо лучше, чем плохо работающая программа вылетающая по исключению.

не боитесь распухания PermGen из-за большого количества классов, ведь ajc для каждого Around метода порождает класс. Или эту проблему как-то можно решить?


Из статьи: По умолчанию аспект создается в виде Singleton.
NullSafeAspect как раз такой.
One instance of the aspect is made. This is the default.

www.eclipse.org/aspectj/doc/next/progguide/quick-aspectAssociations.html

If the aspect has no parent aspect, then by default the aspect is a singleton aspect.

www.eclipse.org/aspectj/doc/next/progguide/semantics-aspects.html

Ajc для каждого Around-метода порождает вставку вызова функции статического внутреннего класса.
http://sidekick.windforwings.com/2009/06/aspectj-through-bytecode-examining.html
Интересно, форматирование кода в примерах — глюки хабрапарсера или задумка автора?
Задумка автора, а что?
Да просто очень тяжело воспринимать эти примеры из-за отсутствия пробелов там, где они всё же должны быть.
Учту на будущее. Сейчас исправить статью невозможно.
Посмотрите на JSR-303 и его реализацию в виде Hibernate Validator. При помощи анотаций javax.validation вы размечаете методы, их параметры и поля классов. Причем набор возможностей валидации гораздо богаче простой проверки на null. Далее методы каждый вызов валидируемого метода прогоняется через валидирующий перехватчик (AOP). Если валидатор обнаруживает проблемы с параметрами/полями, то будет выброшено исключение.
Главное, не забыть всё правильно разметить, нужно вызвать валидатор, нужно сделать код для анализа результата и исправления возникшей проблемы.
А так очень хорошее средство, забыл про него упомянуть.
Тут большое поле для дискуссии — так, например, если вы делаете API для сторонних разработчиков, то увидеть нечто подобное:
public interface MyBeanInterface /** * ..... * @throws ValidationException if value is null or empty */ void setValue(@NotNull @Size(min=1) String value);

намного более предпочтительно по многим причинам:
— javax.validation это документированная и стандартизированная библиотека
— средства валидирования доступны из коробки, они постоянно развиваются и совершенствуются и поддерживаются многими сторонними библиотеками
— повышается наглядность кода и степень его возможного переиспользования
Форматирование пропало. Вот так правильно:

    public interface MyBeanInterface {

        /**
         * @throws ValidationException if value is null or empty
         */
        void setValue(@NotNull @Size(min = 1) String value);
    }
Sign up to leave a comment.

Articles

Change theme settings