Pull to refresh

Comments 49

Интересный подход, спасибо.
В заголовке исправьте на listener.
Спасибо, поправил
Никогда не понимал стремления некоторых java-программистов писать интерфейсы с начальной «I». Неужели мало того, что IDE и так подсвечивает — интерфейс это, класс или абстрактный класс?
Не знаю как у кого, но у меня эта привычка развилась от нежелания смешивать классы и интерфейсы на уровне модели. Дело в том, что модель — она не только в IDE, она еще и в CASE-средствах, на бумаге, в голове, но главное — в разговорах.
Здесь подобная «вербальная семантика» бывает очень полезна (по крайней мере, по моему опыту)
Мне всегда казалось, что вот как раз для модели должно быть все равно — интерфейс у вас, класс или еще чего. У вас есть сущность, с которой внешний мир как-то общается. А чего там внутри — какая разница?
С точки зрения чистой модели — действительно, никакой разницы. Но я говорил не о модели только, но преимущественно — о процессах мышления, осмысления и обсуждения, связанных с этой моделью. А здесь, как мне кажется, разница есть.
Но в целом я согласен, по большому счету это лишь вопрос вкуса и привычки :)
Если это не внешний интерфейс, то у него всегда есть шансы стать абстрактным классом в результате рефакторинга :-) А лишнее переименование при этом — это дополнительная нагрузка на гарантию корректности рефакторинга.
Почему «некоторых» и почему «Java»? Это, вроде как, общепринятое соглашение и в Java, и в .NET, и, думаю, не только в них. Подсветка подсветкой, но удобно видеть сразу, что это именно интерфейс.
В любом проекте программист над какой-то частью кода работает больше, чем над другой. И чем чаще он с ней работает, тем лучше он ее помнит — где у него там классы, а где интерфейсы. А если с этой частью он работает редко — ему все равно необходимо будет посмотреть определение. Так что семантическая примесь к имени не нужна.
Кстати, а Вы в курсе, из-за чего большинство (если не все) программистов перестали использовать венгерскую нотацию? :-)
Ну это вопрос соглашений — они могут быть как хорошими, так и плохими. Мне лично соглашение об именах интерфейсов с I в начале очень даже нравится. Также как и базовый абстрактный класс с суффиксом Base. Это неплохо. Плюс Visual Studio сразу выводит список интерфейсов, когда пишешь «I» :) А венгерская нотация — не знаю, честно говоря, мне лично она не нравится. Ведь имя переменной само по себе содержит ее суть, «intent» и область применения в нужном нам контексте. Причем, речь не только о строго-типизированных языках, но и о динамических.

В конце концов, я думаю, это дело вкуса, но лично меня никак не напрягает «I».

P.S. В качестве маленького бонуса, из API получается fluent-синтаксис :-)
public class Car : ICanRide, IHaveDoors, INeedWheelsToMove { ... }
похоже на тяжелое наследство С++. Там private члены еще с подчеркиванием. Причем где-то с ним, а где-то без него. Ну и от final рябит в глазах.
Наследство C++ у меня и вправду есть, но вовсе не такое тяжелое :)
Что же касается подчеркивания, то оно с успехом применяется и в Java (также как и другие префиксы декларированных полей, типа m_ или f_), причем не противоречит code conventions. Подчеркивание в середине идентификатора — это действительно немного не по-java'овски, но это уж мой личный каприз :) Надеюсь, он не помешал восприятию идеи.
А final — это хорошо везде, где не бессмысленно.
Вот скажите, зачем вам final в private static классе? Или почему бы вам локальные read-only переменные не объявить финальными? Или все сигнатуры методов загадить финалом?

В подавляющем большинстве случаев, final нужет только для объявления публичных полей констант.
А зачем использовать типизированные поля, если можно объявить их как Object?
Модификатор final в объявлении полей (пусть даже и в private классе) используется для указания того, что их значение не должно изменяться — по-моему, вполне нормальная причина.
Для методов final используется чтобы избежать их переопределения в дочерних классах, т.е. в случае, если сам класс объявлен как final или private, то final-методы в нем смысла не имеют (но у меня их и нет).
И вообще, не надо меня обвинять в бесконтрольном использовании final. Все случаи применения final в коде статьи, на мой взгляд, оправданы, а если вам так не кажется — укажите конкретное место в коде. А то заявления типа «от final рябит в глазах» или «загадить финалом сигнатуры методов» не похожи на конструктивную критику. :)
public final class ListenerSupportFactory — финал лишний, т.к. конструктор и так приватный
Здесь же: все поля финал, хотя это бессмысленно — класс приватный, количество кода — на один экран.
Кто это поле может перезаписать кроме вас?

Также советую — искореняйте привычки: interface IInterface, private _privateVarY или private m_privateVarX, int i_came_from_c_or_python

Если читающий не может сразу отличить локальную переменную от члена класса — проблема с кодом, а не с именем члена.

Поймите самое главное, код пишется для людей. Ява итак многословна, не надо еще больше код засорять :) При чтении это только отвлекает, но никакой информации полезной не дает.
Про лишний final при приватном конструкторе — согласен, принимается.
Про то, что код пишется для людей — тоже, конечно, согласен.
По остальному — не согласен, могу поспорить, но не буду, т.к. подобный спор явно не относится к теме статьи. :)
В любом случае, спасибо за замечания!
Я лично использую final с приватным конструктором для утилитных классов и singleton'ов. Удобно тем, что IDE в списке классов помечает final-классы особым значком, что удобно при просмотре дерева иерархии классов.
Собственно у меня две основные ретензии: финал и имена с подчеркиванием для полей класса.
Также в Java можно увидеть использование protected final переменных (не только static!). Это облегчает работу с ними в наследующих классах, но не дает изменять, чтобы не разрушить логику базового класса.
Во-первых, никто не говорил про использование final «где ни попадя».
Во-вторых, приведенная статья сводится к мысли «не используйте final без причины, а причину документируйте». Полностью поддерживаю такую позицию. Соглашусь, что в приведенном в статье коде нет JavaDoc'ов, но это было сделано с умыслом, дабы не отвлекать читателя от основной идеи.
Если я не ошибаюсь, Thread.currentThread().getContextClassLoader() иногда может возвращать null. Рекомендую если null использовать ClassLoader.getSystemClassLoader().

Замечание безусловно интересное, спасибо.

JavaDoc особо эту тему не комментирует, но, судя по тому, что существует Thread.setContextClassLoader(ClassLoader), видимо, можно явно выставить загрузчик текущего контеста в null.
Недолгое гугление навело на информацию о том, что (http://stackoverflow.com/questions/225594/thread-getcontextclassloader-null)
Java threads created from JNI code in a non-java thread have null ContextClassloader unless the creator explicitly sets it.
Also in such context Thread.currentThread() returns null.

Т.е. строго говоря, при некоторых обстоятельствах, не только getContextClassLoader(), но даже Thread.currentThread() может вернуть null.
Полагаю, что программы, рассчитанные на работу в условиях, где подобное возможно, должны предусмотреть собственный метод типа getCurrentValidClassLoader(), в котором аккуратно
обрабатывать все возможные нюансы. В этой же статье добавление подобного обработчика просто
собьет читателей с толку.

Но за замечание все равно спасибо.
Сомневаюсь, что ваш код работает быстрее, чем традиционный подход к реализации паттерна поведения Publish/subscribe.
Так ведь никто и не говорил про «быстрее». Речь шла исключительно об экономии усилий при написании похожего кода
1. Ваш код непотокобезопасен: использование Collections.synchronizedList не даёт вам права итерироваться по списку без синхронизации. Вообще, не факт что стоит делать этот «универсальный слушатель» потокобезопасным, поскольку скорее всего будет иметь место внешняя по отношению к слушателю синхронизация.
2. Махинациями с _current_events вы отсекаете не только циклические вызовы, но и вполне легальные одновременные нотификации из разных потоков.
3. Ловить Throwable — в принципе идея идея не лучшая, а уж пытаться его проигнорировать, отделавшись записью в лог, совсем неправильно. В данном случае вы вполне можете позволить исключению лететь дальше, благо сигнатура метода позволяет.
Насчет 1 и 2 — да, вы правы, спасибо за замечание. Что касается _current_events, то его, вероятно, следует сделать ThreadLocal.
Относительно эе отлова Throwable я не согласен. Тут дело не в стиле программирования, а в логике оповещения подписчиков. Ведь подписчики ничего не знают друг о друге, почему же одни должны «страдать» от кривости других, только потому что они подписались на данное событие позже?
К слову сказать, в процессе написания ListenerInvocationHandler, именно из-за соображений типа п. 3, одно время слушатели из списка вызывались каждый в отдельном потоке. Но на практике это оказалось совершенно crazy, поэтому вернулись к варианту с подавлением исключений.
Можно, конечно, спорить, что лучше ловить — Throwable или Exception, но это в данном случае скорее дело вкуса, чем вопрос по существу.
Это, конечно, ваше право. Может быть в вашем приложении действительно не важно, отработал подписчик или свалился :-)
В моем приложении разработчикам было настоятельно рекомендовано реализовывать подписчики так, чтобы они вообще не выбрасывали исключений, а обрабатывали бы их сами. Это и логично, т.к. подписчик — это по сути автономная подпрограмма. Тому, кто его вызывает, о нем ничего неизвестно и, следовательно, вызывающей стороне глубоко наплевать какие он там исключения генерит или нет (вызывающая сторона чаще всего и не может их обработать, ее дело — только вызвать).
Поэтому здесь отлов Throwable с записью в лог — это «последний барьер защиты» и по-хорошему, конечно, до него дойти не должно.
Такой «последний барьер защиты», если он и есть, должен быть один и находится в main() (ну или run() для потока)
Да, но не в данном случае. Здесь, кроме всего прочего, необходимо защитить одних подписчиков от других. Поэтому исключения тушим именно тут.
Не вижу принципиальных противоречий со статьей. Поясните пожалуйста мысль, если не сложно.
От себя: считаю, что нет никакого криминала в отлове Exception или даже Throwable при определенных обстоятельствах, и, применительно к статье, эти обстоятельства имеют место
жэто не дело вкуса
в аду есть специальное место для тех, кто ловит Throwable! Представьте себе, что Ваш код поймал OutOfMemoryError — что будет?
А что будет с системой в случае ядерного удара?
OutOfMemoryError — это по-любому трындец и перезапуск приложения. Поэтому, отвечая на ваш вопрос, скажу — ничего хорошего не будет, но не будет его и в случае отлова Exception вместо Throwable.
Поверьте, я знаю все «за» и «против», читал это и другие умные статьи и книги. Решение ловить Throwable или Exception — это всегда компромисс. В моем случае, например, подписчики, подключаемые из плагинов, могли иногда выкинуть NoClassDefFoundError. Это, конечно, тоже не дело, но в процессе разработки и экплуатации софта оно визникало на порядки чаще, чем OOME, отсюда и решение — ловить Throwable.
Но конечно, я не призываю всех всегда поступать именно так (да и статья, в общем-то совсем не о том).
ээээ
это трындец и перезапуск если Вы его не поймали, а если Вы его поймали — то это еще несколько суток существования приложения в режиме зомби, пока оно не сломается где-то еще. В случае отлова Exception — у Вас будет хотя бы шанс срегаировать на error и сделать System.exit.

Я согласен, что это не тема статьи, но использование кода с bad smell в публичных примерах это плохо, т.к. новички будут на этом учиться.
Несколько суток в режиме зомби — это все-таки из области программирования серверных приложений, а у меня Swing. В отрыве от контекста — безусловно, отлов Throwable есть зло.
Исправил код в статье, дабы не вводить в соблазн новичков. :)
Основная идея как я понял в использовании шаблона «Декоратор»? (class MyObjectListenerSupport implements IMyObjectListener, IListenerSupport)
Вообще говоря такие задачи стандартны(сводятся к минимизации повторения кода) и в большинстве своём уже реализованы.

И естественно, если еще не прочитаны книги про шаблонам проектирования — советую прочитать, очень полезно.
UFO just landed and posted this here
Нет, намного.
Кроме необходимости писать метод типа fireXXXX(...) на каждый метод интерфейса слушателя, данный кусок кода не решает, например, вопросов циклического вызова слушателей или защиты механизма оповещения от плохо написанных слушателей. Добавьте эти проверки в метод fireEvent(Event), представьте что слушатель подразумевает десяток событий, а observable-объектов у вас тоже пара десятков. Вот и получите тонны одинакового кода.
UFO just landed and posted this here
Посмотрел. Действительно интересно, спасибо за ссылку!
По сути Announcer — это то же самое, только он не сам является прокси, а агрегирует ее. Не очень понятно только, зачем там реализация InvocationHandler перенесена, по сути, в сам Announcer (я бы вместо анонимного сделал inner класс и всю реализацию в нем), но это несущественно.
1. Java не умеет множественно наследовать функциональность, поэтому приходится извращаться либо с Proxy, либо громоздить с delegate. В Scala это делается очень изящно при помощи трейтов. Так что есть смысл переходить на Scala.

2. > Необходимо позаботиться о многопоточном использовании observable-объекта
Отделите мух от супа. Многопоточная архитектура имеет совершенно иную специфику: синхронные и асинхронные события, блокирование, коллизии и повторы. В этому случае Вам подойдет модель актеров (Actors) akka.io
А для десктопной апликации не стоит так извращаться.

3. В Вашей реализации есть одна типичная ошибка: если обзервер, получив событие, больше не хочет принимать, он дерегистрирует себя из объекта-источника. В таком случае ваш цикл отсылки
for (T listener: _listeners) {
вылетит с ConcurrentModificationException, т.к. список _listeners был изменен во время прохода. Нужно проходить по копии _listeners.

4. Вообще модель продьюсер-обзервер плоха не столько количеством лишнего кода, сколько необходимостью строгого контроля регистрации/дерегистрации обзерверов. Это есть корень всех ошибок с memory leaking в программах Java. Java девелоперы, привыкшие к GC, редко заботятся об освобождении ресурсов, в том числе и дерегистрации обзерверов, которые так и болтаются в списке. Друкая проблема — запутанность логики, структуры и зависимостей. Поэтому все чаще в проектах используют EventBus как замену архитектуре producer-observer, напр. www.eventbus.org/
Я эту проблему на Дельфи решил следующим образом: метод подписки возвращает регистрационную метку («гардеробный номерок») — интерфейсную ссылку, очистка которой приведет к отписке. Если ее хранить в поле объекта-подписчика — отписка при его удалении произойдет автоматически. Плюс не нужен специальный метод отписки.
У Вас «check-then-act» проблема когда Вы ивенты проверяете на вхождение

if (_current_events.contains(methodName)) {
...
}
_current_events.add(methodName);



Often problems occur when one thread does a «check-then-act» (e.g. «check» if the value is X, and then «act» to do something that depends on the value being X) and another thread does something to the value in between the «check» and the «act».

Решение может служить замена Collections.synchronizedSet(new HashSet()); на конкурентную мапу (ConcurrentHashMap), в которой храните ивенты как ключи и null как значения
Ну предыдущий код можно записать вот так… чтобы было более понятно, где чек и где акт)

if (!_current_events.contains(methodName)) {
_current_events.add(methodName);
}esle{
throw new RuleViolationException();
}
Да, все верно, код не потокобезопасный, romik об этом уже сказал выше.
Sign up to leave a comment.

Articles