Final Or Not Final? That Is The Questuion.

whirlwind

TDD infected, paranoid
Всё что я хочу, указав final, — это сказать, что этот класс наследовать не нужно и стоит найти другой способ.
Любой разработчик должен выполнять свою задачу, а не мою. Я и сам в состоянии решить что мне больше подходит в данном случае, а что нет. Существует множество причин, по которым именно этот метод не может быть выделен в отдельную сущность прямо сейчас (правило трех ударов, например). И почему собственно ты настаиваешь, что этот класс наследовать не стоит? Складывается впечатление, что не все в порядке в твоем классе. Может инкапсуляция нарушена или еще что то.

PS. Я могу сказать когда final нужен однозначно. С final-ом на переменные компилятор/VM точно знает, что с этими данными можно работать без блокировок и межпоточной синхронизации.
 

Вурдалак

Продвинутый новичок
Любой разработчик должен выполнять свою задачу, а не мою. Я и сам в состоянии решить что мне больше подходит в данном случае, а что нет.
С таким же успехом можно сказать, что «никогда не ставьте private, только protected или даже public, я уж как-нибудь и без вас разберусь что дёргать и переопределять». Я стараюсь никогда не делать оптимизации для менее квалифицированных разработчиков, предоставляя им удобные возможности для говнокода. Я, как автор API этого модуля, имею полное право закрыть его от наследования, если считаю это разумным, потому что мне, например, поддерживать BC.

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

Более того, наследование совсем не гарантирует того, что у тебя получится расширить поведение должным образом: вполне возможно, что нужная точка будет где-то в середине метода. И тут либо редактировать исходный класс, предоставляя ещё один метод для переопределения, либо сделать уже наконец по-нормальному. И тут по времени не будет практически никакой разницы. UPD: а, ну ещё есть вариант с полным копированием исходного метода и «исправлением». Это по сути то же самое, что и копировать класс, только, на мой взгляд, опаснее, т.к. не будет вызова parent-метода, что чревато проблемами.

Подброшу немного argumentum ad verecundiam:
http://verraes.net/2014/05/final-classes-in-php/
http://ocramius.github.io/blog/when-to-declare-classes-final/
 
Последнее редактирование:

whirlwind

TDD infected, paranoid
Потому что я не вижу ни одного валидного кейса, когда это может быть сделано, так зачем мне поддерживать BC для говнокодеров?
А, ну если не видишь валидного кейса, то это другой разговор. Это только говорит о том, что ты мало сталкивался. Когда раза три фасады с большим кол-вом методов туда сюда с декораторов на наследование перепишешь, или когда на дейли-митингах начнете обсуждать нехватку адекватных имен, или оверхед в части дублирования одного и того же состояния из-за высокой декомпозиции (переливание атрибутов одного класса в атрибуты выделенных и распухание кода на этом), тогда и валидные кейсы найдутся.

PS. Кидаться ссылками на сомнительные коротенькие статьи не стоит. На простых примерах всегда все очень красиво, это я давно говорил. Хорошие примеры это работа на стыках классов и наличие модульных тестов. Пока модульных тестов нет, нет и понимания важности полиморфизма. Да, я избегаю наследования, потому что любое наследование приводит к дублированию кода теста. Но даже при всем этом, final на методы это зло.
 
Последнее редактирование:

Вурдалак

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

Когда раза три фасады с большим кол-вом методов туда сюда с декораторов на наследование перепишешь
Не делай фасадов с большим количеством методов, не нарушай ISP.

или когда на дейли-митингах начнете обсуждать нехватку адекватных имен
В этом тоже виноват final? Мне кажется, что ты начинаешь сочинять сказки, лишь бы оправдать implementation inheritance.

Пока модульных тестов нет, нет и понимания важности полиморфизма.
Закрытие от наследования не означает нарушение OCP, не забывай.
 

fixxxer

К.О.
Партнер клуба
Эк вы развели.

Я вот честно скажу: я не ставлю final, потому что иногда надо по-быстрому наговнокодить :)
 

whirlwind

TDD infected, paranoid
Я боюсь, что это слабый аргумент, можешь бить себя в грудь и говорить, что ты великий, но это действует наоборот.
Это не аргумент. Это тебе напоминание, что отсутствие доказательства само по себе доказательством не является. Если ты не видишь суслика, это не значит что его нет. Ты не привел ни одного практического примера, когда final вредит.

Не делай фасадов с большим количеством методов, не нарушай ISP.
Еще раз. Не нужно мнить себя гением и указывать как я должен работать. В этом основная претензия по поводу final. Практика критерий истины. Ограничениями ты ограничиваешь практику применения. Ты не можешь представить все возможные варианты использования своего класса. И правильный подход не делать прогнозов вовсе, не внося искусственные ограничения. final-ом ты диктуешь дизайн пользовательского кода. Это плохая практика, если ты хочешь что бы твой код был хорошо реюзабельным.

В этом тоже виноват final? Мне кажется, что ты начинаешь сочинять сказки, лишь бы оправдать implementation inheritance.
final виноват в том, что если на каждый чих ты будешь плодить новую сущность, очень быстро реальные задачи потонут в обсуждении проблем управления кодом. Ну нет никакой необходимости выделять интерфейс для единичного случая декомпозиции внутри пакета исключительно для того, что бы нормально протестировать. И таких классов в моих проектах подавляющее большиство, в отличии от предназначенных для публичного использования.

Закрытие от наследования не означает нарушение OCP, не забывай.
А я не согласен с этой формулировкой. Она не имеет смысла вне контекста минимального языковой единицы - класса. С таким же успехом можно утверждать о программе в целом. Нет такой программы, которую невозможно переписать. Для меня очевидно, что под OCP подразумевается объем прилагаемых усилий, а не принципиальная возможность изменений, которую никто не оспаривает.
 

Вурдалак

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

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

Ты не можешь представить все возможные варианты использования своего класса.
Я лишь отсеиваю заведомо неправильные.

Это плохая практика, если ты хочешь что бы твой код был хорошо реюзабельным.
На практике, разницы быть не должно. Ещё раз, особенно касаемо внутренного проекта: не хватает возможности — добавь точку расширения через конструктор вместо наследования. Различие только в способе расширения.

final виноват в том, что если на каждый чих ты будешь плодить новую сущность, очень быстро реальные задачи потонут в обсуждении проблем управления кодом.
Откуда у тебя появляются «проблемы управления кодом»? Как правило, существует набор гласных и негласных соглашений, который помогает с неймингом и прочими вещами, это делается на автомате. Правда на ревью я сам люблю придираться к именованию, но обычно компромисс быстро находится.

Ну нет никакой необходимости выделять интерфейс для единичного случая декомпозиции внутри пакета исключительно для того, что бы нормально протестировать. И таких классов в моих проектах подавляющее большиство, в отличии от предназначенных для публичного использования.
Вот этого я не понял, что ты имеешь в виду? Если ты имеешь в виду мок конкретного класса, то тут 2 проблемы: одна из них, соглашусь с Verraes, инфраструктурная: ну, если очень нужно, то можно и без final в явном виде, а с phpdoc-тегом @final; вторая — теоретическая, зачем тогда мок, почему не инстанс конкретного класса?
 

whirlwind

TDD infected, paranoid
На практике, разницы быть не должно. Ещё раз, особенно касаемо внутренного проекта: не хватает возможности — добавь точку расширения через конструктор вместо наследования. Различие только в способе расширения.
Наверное я плохо объяснял на пальцах. Попробую на конкретном примере. Имеем достаточно большой фасад, представляющий третью сторону. То что тут надо чего то делить и разбивать не обсуждается ибо в этом и был весь смысл, что бы сократить зависимость от третьей стороны.

Код:
  @Override
   public void placeOrder(Order order) throws OrderException {
     synchronized ( order ) {
       OrderStatus status = order.getStatus();
       EditableOrder o = (EditableOrder) order;
       if ( status == OrderStatus.PENDING ) {
         OrderActivator activator = o.getActivator();
         if ( activator != null ) {
           activator.start(o);
           o.setStatus(OrderStatus.CONDITION);
           orders.fireEvents(o);
           return;
         }
       } else if ( status == OrderStatus.CONDITION ) {
         order.getActivator().stop();
       }
       orderProcessor.placeOrder(this, order);
     }
   }
В данном случае orderProcessor это специально выделенный интерфейс как и положено. За этим интерфейсом специфика конкретной части, то бишь работы с заявками. Что бы разорвать циклическую ссылку, обеспечить целостность и согласованность сразу после инстанцирования конкретной реализации фасада и при этом обеспечить возможность реализовать алгоритм любой сложности, метод placeOrder принимает инстанс фасада. Здесь нет смысла мудрить, потому что мы изначально не знаем какая специфика скрывается за конкретными реализациями. Мы не можем выделить никакой интерфейс для передачи в процессор, потому что это уже спецификация протокола, который нам, повторяю - неизвестен.

Если этот базовый фасад объявить final, то этот код уже не будет работать коректно. Ведь если его декорировать, то this это будет инстанс базовой реализации, а в процессоре нужна специфическая реализация. Здесь можно вывернуться, добавив публичный метод типа setTopInstance, который фабрика фасада должна будет вызвать после инстанцирования своего специфического инстанса. Но это сразу архитектурная проблема и не одна. Во первых, это будет публичный метод, так как декорировать можно только публичные методы. И он будет торчать наружу там, где эти кишки абсолютно не нужны, а только ведут к потенциальным проблемам использования. Далее, велик шанс что называется забыть его установить после инстанцирования. В третьих, это нестандартный подход будет постоянно напрягать необходимостью использовать getTopInstance везде внутри самого фасада. Я надеюсь этого достаточно, что бы выкинуть отсюда final.

И в этом случае отлично работает наследование и расширение. Сам фасад не содержит много кода, он содержит много методов-делегатов к подсистемам. И служит отличной абстракцией, которая позволяет реализовывать различные подсистемы за фасадом в виде отдельных несвязанных классов. В случае с файналом пришлось бы связывать эти подсистемы между собой каждый раз новым специфическим способом. А так, мы имеем отличную тестируемую архитектуру. Она уже разделена на подсистемы и тут нечего выделять в новый интерфейс, потому что задача обратная - собрать пакет в единую систему.

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

вторая — теоретическая, зачем тогда мок, почему не инстанс конкретного класса?
Я не отношусь к тем людям, которые твердо уверены, что из двух существующих: тестирование поведения и тестирование состояния - только один подход является правильным. Очевидно, что для разных классов эти два подхода будут давать различную эффективность. Но ты прав, это отдельная большая тема.
 
Последнее редактирование:

Вурдалак

Продвинутый новичок
Во-первых,
Код:
@Override
   public void placeOrder(Order order) throws OrderException {
     synchronized ( order ) {
       inner.placeOrder(order);
       orderProcessor.placeOrder(inner, order);
     }
   }
не? Кстати говоря, я не понимаю почему ты зациклился на декорировании, не забывай про всякие listeners, observer и т.д.

Во-вторых, взаимодействие с 3-й библиотекой (т.е. адаптеры), на интерфейс которой я не могу повлиять, я всегда отношу к «грязному» коду, где я могу себе позволить говнокод той степени, который диктует и заслуживает сама библиотека.
 
Последнее редактирование:

whirlwind

TDD infected, paranoid
Во-первых,
Код:
@Override
   public void placeOrder(Order order) throws OrderException {
     synchronized ( order ) {
       inner.placeOrder(order);
       orderProcessor.placeOrder(inner, order);
     }
   }
не? Кстати говоря, я не понимаю почему ты зациклился на декорировании, не забывай про всякие listeners, observer и т.д.
Это ты предлагаешь в каждом перекрываемом методе так делать? Ну оно примерно так и было. Только не пойму где тут профит.

И события использую. Только этот конкретный архитектурный вопрос событиями красивее не решается. Собственно, можешь глянуть на этот базовый фасад http://pastebin.com/9jy0Eu5q кода там кот наплакал. одни делегаты.

Во-вторых, взаимодействие с 3-й библиотекой (т.е. адаптеры), на интерфейс которой я не могу повлиять, я всегда отношу к «грязному» коду, где я могу себе позволить говнокод той степени, который диктует и заслуживает сама библиотека.
Ну предположим не библиотека, а некий протокол, работу с которым ты полностью сам реализуешь. Смысл в том, что подсистемы между собой тоже по определенному протоколу общаются.

Вообще декоратор не такой простой паттерн, как может показаться. Злоупотреблять им определенно не стоит.
 

Вурдалак

Продвинутый новичок
Код:
        public BasicTerminal(BasicTerminalParams params) {
                super();
                this.controller = params.getController();
                this.dispatcher = params.getEventDispatcher();
                this.securities = params.getSecurityStorage();
                this.portfolios = params.getPortfolioStorage();
                this.orders = params.getOrderStorage();
                this.starter = params.getStarter();
                this.scheduler = params.getScheduler();
                this.es = params.getEventSystem();
                this.orderProcessor = params.getOrderProcessor();
        }
Вот что тот код на плюсах, что этот — один большой-большой класс с набором несвязанных методов. А потом «у меня фасады с большим кол-вом методов».

Если весь твой код — это набор таких больших классов, то это печально; если нет, то всё равно в большинстве случаев можно поставить final.
 

whirlwind

TDD infected, paranoid
Если честно, совсем не понял о чем твой последний комментарий. Этот фасад - стандартизированная модель торговой системы. Абстракция по всем заветам ООП. То что он делегирует конкретным подсистемам говорит о том, что утверждение "весь мой код это набор таких больших классов" очевидно не соответствует дейсвительности, как минимум классы "поменьше" там есть, если это для тебя главный критерий. Мы можем обсудить каждую строчку или подход, проблему и решение. Я так и не понял, что конкретно тебя не устраивает в этом коде и где тут можно поставить final что бы это получилось лучше по всем параметрам?

Тебе непонятно для чего сущность BasicTerminalParams введена? Объясняю: что бы защитить потребителей от изменений при модификации базового фасада. Что бы не фиксить все имеющиеся реализации терминалов и всех потребителей в случае необходимости инжектнуть новый компонент. Это делается через такой параметр + билдер. Параметр, билдер и сам базовый фасад лежат в одном пакете и обновляются одновременно.

http://pastebin.com/H6E3MMFD

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

Бери на вооружение ;)

PS. Мы можем обсуждать этот код долго. Например, почему выбран constructor injection и почему все поля final. Но это не имеет никакого отношения к обсуждению OCP или final на классы/методы. Никаких убедительных примеров в пользу final я к сожалению здесь до сих пор не увидел.

PPS. Еще обрати внимание, чем хороша предложенная мной архитектура. Это то, о чем я говорил, о недопустимости диктата дизайна с более низкого уровня, на котором ты настаиваешь. Смотри, мой код абсолютно никак не ограничивает архитектуру потребителей. Ты можешь использовать абсолютно любой подход, который сочтешь наиболее приемлимым для себя. Например, ты можешь декрнуть все и вся. Ты можешь инкапсулировать базовый билдер в билдере конкретного терминала как это показано в последнем пастбине. Ты можешь даже унаследовать параметр и расширить его своими атрибутами. Ты можешь использовать любой подход, который я забыл перечислить. Все это будет работать. Пользоваться этими классами должно быть легко и приятно.
 
Последнее редактирование:

Вурдалак

Продвинутый новичок
whirlwind, твой фасад нарушает SOLID (в частности, ISP и SRP), его можно разбить на command + command handler (placeOrder, cancelOrder, ...), отдельные listeners (OnConnected, OnDisconnected, ...), etc. Но сейчас не это главное: ты к нему прицепился, говоря, что final тут будет мешать. Ты его спроектировал специально таким образом, чтобы final был неудобен, OK, не ставь в этом конкретном случае final. Что же по поводу остальных 99% классов?

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

whirlwind

TDD infected, paranoid
Ребят, я честно не врубаюсь в чем у вас непонятки. Что конкретно странновато выглядит и почему command тут будет лучше? Самое непонятное - чем лучше оно будет размазывание соплей по классам, там где нужна одна единая точка доступа для потребителей сервиса? У меня стандартная нормальная архитектура торговой платформы. Абстракция - торговый терминал. Куча аналогов по такой же архитектуре примерно. Вот например на шарпе http://stocksharp.com/doc/html/T_StockSharp_BusinessEntities_IConnector.htm При чем здесь можно разбить или нельзя? Вот честно пытаюсь понять о чем толкуем, но не могу. Где конкретно какой принцип нарушен показывайте и объясните подробно в чем нарушен и почему это плохо.

Вот черным по белому в той же википедии написано

Шаблон фасад (англ. Facade) — структурный шаблон проектирования, позволяющий скрыть сложность системы путем сведения всех возможных внешних вызовов к одному объекту, делегирующему их соответствующим объектам системы.
У меня так? Так! В чем вопрос тогда? Реально не понимаю, кто из нас не в теме :)

PS.

> когда ты сам уверен в том, что наследование тут будет неприемлемо

Не бывает таких ситуаций, если это не inner class. А для inner есть другие средства.
 
Последнее редактирование:

Вурдалак

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

Во-вторых, определение Фасада не противоречит тому, что твой класс нарушает SOLID. Вообще говоря, паттерн сам по себе не может быть аргументом того, что ты всё делаешь правильно :) Singleton тому пример.

В-третьих, фасад ставит своей целью «скрывать сложность системы», но у тебя я вижу обилие getter'ов, которые сводят это «скрытие» на «нет», например: у тебя есть BasicTerminal.getOrderProcessor(), так что мешает взять и заюзать его в обход placeOrder() тем самым, вероятно, введя систему в неконсистентное состояние?

В-четвертых, допустим, у тебя вот есть placeOrder(), у него я не вижу ни одного вызова protected-методов, тогда в чём состоит твой основной аргумент, что тебе может потребоваться наследование? Что ты у конкретно этого метода собираешься менять в поведении и как?

Не бывает таких ситуаций, если это не inner class. А для inner есть другие средства.
Возьми любой класс с одним методом, полная перегрузка метода будет эквивалентна другой реализации.
 

fixxxer

К.О.
Партнер клуба
Может, я неправильно понимаю паттерн, но я всегда считал, что фасад - это не средство инкапсуляции, а просто сервисный класс, сводящий все API к методам одного класса, этакий front controller уровня библиотеки. Хочешь делать в обход - да пожалуйста, никакой неконсистентности не должно быть.

UPD: Ну, то есть, я фасад всегда пишу в последнюю очередь, и его использование всегда опционально, а при проектировании вообще не учитываю его наличие.
 
Последнее редактирование:

Вурдалак

Продвинутый новичок
Может, я неправильно понимаю паттерн, но я всегда считал, что фасад - это не средство инкапсуляции, а просто сервисный класс, сводящий все API к методам одного класса, этакий front controller уровня библиотеки. Хочешь делать в обход - да пожалуйста, никакой неконсистентности не должно быть.

UPD: Ну, то есть, я фасад всегда пишу в последнюю очередь, и его использование всегда опционально, а при проектировании вообще не учитываю его наличие.
Хочешь делать в обход — пожалуйста, только зачем делать геттеры у фасада? Впрочем, это, пожалуй, к возможной неконсистентности не относится, согласен.
 
Последнее редактирование:

fixxxer

К.О.
Партнер клуба
Эм, ну я не знаю, зачем там геттеры. Мне их наличие тоже подозрительно, таким макаром фасад может превратиться в god object.
 

Вурдалак

Продвинутый новичок
Опять меня спровоцировали на обсуждение какого унылого кода. К разговору о final это не относится, final хорош при достаточном уровне декомпозиции, чего здесь не наблюдается; этот класс даже своим названием (Base...) намекает на возможность наследования, что ставит его в один ряд с abstract-классами. При достаточном уровне декомпозиции классы будут маленькими, реализации можно закрывать от наследования. Если у тебя большие классы и ты делаешь их final'ами, то это твоя вина, а не самого final, тогда обломись и продолжай inheritance driven development, я не настаиваю.
 
Сверху