Расширение класса внешними методами и переменными

whirlwind

TDD infected, paranoid
Не знаю насчет Вурдалака, у меня и такие, и такие, в зависимости от. Раз ты уж так привязался к show me the code, ну вот такая фигня на гитхабе валяется.
Я сильно углубляться не буду, но вот смотри по верхам. RouteDispatcher хороший пример демонстрации отсутствия проблемы. Есть у тебя контроллер. Под ним нет никакой инфраструктуры. Для него по сути фабрика нужна что бы по имени класс инстанцировать - все. Где DI тут? Полиморфизм не нужен. С другой стороны, роутер не требует контракта от контроллера. То есть, с этой стороны полиморфизм тоже не нужен. Отсюда следует, что вы там все можете зафайналить и никто не пострадает.

Дальше. Честный тест на dispatch не сделан. Посчитай количество взаимоисключающих ветвей внутри форыча и возведи в квадрат минимум (минимум 2 элемента = 2 прохода), что бы обозначить работу цикла. Что бы тестить жирные циклы нужно тело цикла выносить за контрактный метод (назовем этот контракт хелпером) и отдельно тестировать 2 кейса: когда код идет по циклу, а отдельно тестить один проход по циклу (тест логики обработки элемента). Если обе части покрыты на 100%, значит и в комбинации они работают правильно. Но общий тест без выноса будет очень сложен, что и демонстрирует твой тест. И смотрим опять: DI нет, 100% покрытия нет. Нет DI, нет понимания что это внутренний дизайн и лепить тут implements на класс логики обработки элемента бессмыслено. Этот хелпер класс должен быть package visible, а юзер (диспатчер в данном случае) будет предусматривать два конструктора: package visible с возможностью инжектнуть мокнутый хелпер и дефолтный конструктор, который инстанцирует реальный хелпер.

Ну и так далее по тексту.
 

fixxxer

К.О.
Партнер клуба
Что ты понимаешь под DI? Контейнера конечно нет, делать контейнер на такую микробиблиотеку бессмысленно. Есть фабрика из одного метода, которая просто-напросто заменяется на другую (или вообще на Laravel-овский DIC), если надо подсунуть другие реализации. А DIP соблюдается.

Что касается RouteDispatcher::dispatch - это ты хорошо подметил самое архитектурно проблемное место. В первой версии цикломатическая сложность там была куда ниже, и такой дизайн выглядел более уместно (даже идея подсунуть туда другой диспетчер не выглядела так глупо - идея была в том, что кому-то может понадобиться вообще другой вид диспетчеризации, не по принципу controller-method), а потом мне было уже лень заниматься декомпозицией. Тесты из-за этого получились переусложненными, но они, если приглядеться, неплохо покрывают все это разнообразие. По крайней мере, все серьезные проблемы в ходе того дописывания они успешно ловили.
 
Последнее редактирование:

fixxxer

К.О.
Партнер клуба
юзер (диспатчер в данном случае) будет предусматривать два конструктора: package visible с возможностью инжектнуть мокнутый хелпер и дефолтный конструктор, который инстанцирует реальный хелпер
Вот не люблю я такого рода дизайн, когда метод существует только ради тестов. Это становится частью контракта, и получается не пойми что - висит груша, нельзя скушать. Не, либо трусы, либо крестик. Я не спорю с тем, что декомпозиция там нужна, но я бы ее делал не так.

Ну и какой package visible в php? :) Только с анонимными классами если извращаться, но это как-то совсем уж.
 
Последнее редактирование:

whirlwind

TDD infected, paranoid
Это становится частью контракта
Нет! Контракт - это паблик интерфейс. Например inner classes в Java как раз для таких целей - дать поле для маневра при сохранении инкапсуляции. Ну сделай protected ctor или фабричный метод. Это уже детали. Было бы желание.

DI это не обязательно IoC контейнер, DI это когда ты пушиш зависимости снаружи вовнутрь, а не пуллиш их изнутри (как в случае SL). У IoC и SL свои тонкости, но то отдельная большая тема. И там и там ключевой момент - полиморфизм. Если один объект не может прикинуться другим, то LSP не работает. А интерфейсы и абстрактные классы - это вообще чисто локальная языковая фишка.
 
Последнее редактирование:

fixxxer

К.О.
Партнер клуба
DI это не обязательно IoC контейнер, DI это когда ты пушиш зависимости снаружи вовнутрь
Я так и делаю. Или ты до FormRequestFactory докопался? Да забей на него, это вообще для одного чувака с гитхаба сделал за полчаса, это банальная обертка вокруг Laravel-овской инфраструктуры, считай, что inner class. Можно было бы, конечно, вынести в аргумент конструктора или сделать отдельный named, ну, да, но там это не роляет вообще. Она там сама по себе тот еще костыль.
Например inner classes в Java
Если бы у бабушки была Java...
Ну сделай protected или фабричный метод. Это уже детали.
Да все равно это все доступно становится для переопределения, пусть чуть большими усилиями. Не хочу такое давать без полноценной декомпозиции - получится как бы точка расширения но при этом с тесной связанностью. Будут потом всякие идиотские issues мне писать. :) Нормальная декомпозиция там возможна, но требует усилий, которых эта либа просто не заслуживает. Существующий тест все комбинации покрывает на самом деле, просто это неочевидно из-за сложности.
 
Последнее редактирование:

whirlwind

TDD infected, paranoid
Мы все всегда говорим о том кто чего не заслуживает. 100% честный блэкбокс тест на void метод с одним int аргументом должен быть вызван для 2^32 значений. Но никто так не делает, потому что оно того не стоит. Тоже самое с файналом. Польза от него крайне сомнительна, а вред огромен. Очешуенная, популярная и всем известная либа gson охрененна всем, кроме того, что с тестами придется изрядно заморочиться. Ибо какой-то умник сделал final на фасад который не имплемент. Примеры вреда я могу приводить бесконечно ))
 

fixxxer

К.О.
Партнер клуба
Ну где ты у меня там final без implement нашел-то, а? :)

Проблема RouteDispatcher (которую ты хорошо отметил) там вовсе не в final, а в отсутствии декомпозиции. Убрать final и поменять private на два protected-а - это не решение, это костыль чисто для тестов. Если кто-то решит от него унаследоваться, это будет полным фаталити - придется обеспечивать BC для всей этой внутрянки.
 

whirlwind

TDD infected, paranoid
Я не про тебя, а в общем. В идеале про final я не уверен, а в среднем - он вреден. Вы же утверждали что оно панацея. Когда покажете тесты уровня 2^32 на 1 int аргумент, тогда соглашусь ))) А то моду взяли, как тесты писать так можно пофилонить, а как стереотипы распространять - так сразу прям идеальный у вас код )))
 

fixxxer

К.О.
Партнер клуба
И, да, про inner classes - в том же Typescript я так часто делаю. А в PHP при соблюдении DIP так или иначе вся "внутрянка" становится публичной. Тут либо излишняя декомпозиция, чтобы предоставлять стабильное API, либо вот так. Что поделать, PHP говно))
 

fixxxer

К.О.
Партнер клуба
Когда покажете тесты уровня 2^32 на 1 int аргумент
Вот, знаешь, видел я такие тесты. И все сводилось к тому, что сначала менялся код, а потом правились тесты, чтобы не падали))

Тут тоже надо с умом подходить, а не молиться богу покрытия.
 

Yoskaldyr

"Спамер"
Партнер клуба
Пока вирусная паника под шумок этот трешовый rfc кажется примут
write-once пропертис которые будут называться readonly - просто фейспалм
 

fixxxer

К.О.
Партнер клуба
Mixed feelings.

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

С другой стороны, возможность присвоить один раз даже снаружи - это какой-то facepalm.
 

fixxxer

К.О.
Партнер клуба
это очень странный lazy load, для которого надо делать unset в конструкторе (!) и извращаться с __get. Уж проще геттером.
 

WMix

герр M:)ller
Партнер клуба
для которого надо делать unset в конструкторе (!)
о чем ты? зачем unset?
Another restriction of “write-once” properties is that they can't have a default value. Thus, the following syntax is forbidden:
PHP:
class Foo {
    <keyword> public int $a = 0;
}
я так понял подразумевается это
PHP:
$obj = new Foo();
$obj->a = 42;
 

Yoskaldyr

"Спамер"
Партнер клуба
Another restriction of “write-once” properties is that they can't have a default value. Thus, the following syntax is forbidden:
это кстати еще одна причина, почему данная реализация треш - на выходе из конструктора получаем невалидный объект (даже значение по умолчанию не сделать)
 
Сверху