Constructor injection VS Setter Injection - флейм из плейсхолдеров

Breeze · 25 Авг 2013

Фанат написал(а):
Я, правда, так и не понял, как разделить парсер и либу. что передавать из парсера? Объект? Как это всё соединять потом?

я так понимаю он предлагает сделать что-то типа

PHP:

// псевдокодик
$conn = new MyDBLib( new MyPlaceholdersParser() );

а либа пусть сама определяет, что ей делать с результатами парсера

grigori · 25 Авг 2013

Фанат написал(а):
Я, правда, так и не понял, как разделить парсер и либу. что передавать из парсера? Объект? Как это всё соединять потом?

например:
1. Class SafeDB вызывает парсер Class SafeDbParser,
2. SafeDbParser парсит строку запроса с плейсхолдерами и возвращает SafeDbParameters, который знает какие в запросе есть имена и типы
плейсхолдеров
3. SafeDB отдает данные пользователя в SafeDbParameters
4. SafeDbParameters обрабатывает проверяет данные и приводит к нужным типам
5. SafeDB вызывает SafeDbQuery
6а. SafeDbQuery подставляет в строку запроса данные из SafeDbParameters
6б. SafeDbQuery приводит строку запроса с именованными плейсхолдерами к нативным плейсхолдерам PDO/MySQLi/Postgres
SafeDbQuery возвращает объект PDO или MySQL с прибайндиными данными
7. SafeDB вызывает вызов PDOStatement->execute() или mysqli_stmt->execute()
8. Результат запроса может оборачиваться в пользовательский класс или объект, который передается параметром в SafeDB

дальше с результатом запроса работает приложение или фреймворк

grigori · 25 Авг 2013

Breeze написал(а):
я так понимаю он предлагает сделать что-то типа

PHP:

// псевдокодик $conn = new MyDBLib( new MyPlaceholdersParser() );

нет, я предлагаю сделать что-то типа

PHP:

interface iSafeDbParser{
}
class SafeDb{
protected $parser;
setParser(iSafeDbParser $P){$this->parser = $p;}
getParser(){return $this->parser? : new SafeDbParser;}
}
$DB = new SafeDb;
$DB->setParser(new MyParser);

ибо tell don't ask

Вурдалак · 25 Авг 2013

grigori написал(а):
нет, я предлагаю сделать что-то типа

PHP:

interface iSafeDbParser{ } class SafeDb{ protected $parser; setParser(iSafeDbParser $P){$this->parser = $p;} getParser(){return $this->parser? : new SafeDbParser;} } $DB = new SafeDb; $DB->setParser(new MyParser);

ибо tell don't ask

Через конструктор по-любому лучше. А причём тут tell don't ask — grigori его знает.

AmdY · 25 Авг 2013

grigori написал(а):
например:
1. Class SafeDB вызывает парсер Class SafeDbParser,
2. SafeDbParser парсит строку запроса с плейсхолдерами и возвращает SafeDbParameters, который знает какие в запросе есть имена и типы
плейсхолдеров
3. SafeDB отдает данные пользователя в SafeDbParameters
4. SafeDbParameters обрабатывает проверяет данные и приводит к нужным типам
5. SafeDB вызывает SafeDbQuery
6а. SafeDbQuery подставляет в строку запроса данные из SafeDbParameters
6б. SafeDbQuery приводит строку запроса с именованными плейсхолдерами к нативным плейсхолдерам PDO/MySQLi/Postgres
SafeDbQuery возвращает объект PDO или MySQL с прибайндиными данными
7. SafeDB вызывает вызов PDOStatement->execute() или mysqli_stmt->execute()
8. Результат запроса может оборачиваться в пользовательский класс или объект, который передается параметром в SafeDB

дальше с результатом запроса работает приложение или фреймворк

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

hell0w0rd · 25 Авг 2013

grigori
ничего не бред. Можно сделать как в твиге - класс расширения, правда там это оправдано, ибо можно встроить функцию/фильтр/блок и чего-то там еще, а тут только тип плейсхолдера.
То есть если $db->setParser в разных точках приложения будет меняться - ок, а если в разных точках приложения $db->addParseType - то вешаемся? Хочется "расширяемого" парсера - берите доктрину, вот там парсер так парсер

Вообще чем больше будет становиться классов в этой библиотеке - тем больше их объекты будут друг на друга ссылаться и как верно подметил Amdy получится вторая доктрина)

grigori · 26 Авг 2013

AmdY ты так пишешь, будто доктрина - это плохо

предлагай решение для расширяемости или обоснуй почему она не нужна вообще, очерки по философии неинтересны

hell0w0rd, twig - понятный пример, но я не вижу связи между крупным проектом движка шаблонизатора с маленькой либой для типизации плейсхолдеров

grigori · 26 Авг 2013

Вурдалак написал(а):
Через конструктор по-любому лучше.

чем? я считаю, что меньше параметров у конструктора - удобней API

А причём тут tell don't ask — grigori его знает.

tell don't ask - концепция, по которой класс не должен запрашивать параметр у другого объекта, он должен получить его извне,
аналогично, не надо ждать, что в класс передадут значение в конструкторе, лучше позволять переопределить значение, и этим уменьшить связанность, упростить API

grigori · 26 Авг 2013

сейчас мы опять вернемся к той старой теме, что под MVC каждый понимает свое, 2/3 никогда не читало банду 4х, а кому-то главное в жизни - тесты, и ниинтересует

AmdY · 26 Авг 2013

grigori
нет, ты что, доктьрина не плоха, она просто ужасна, огромный оверхед как по производительности. так и по количеству кода. Там весь функционал отталкивается от требований малюпасенького количества проектов, при этом неизвестно нужна ли доктрина самим этим проектам. К тому же она уже есть и повторять незачем, как и делать symfony из yii.

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

hell0w0rd · 26 Авг 2013

А при чем тут мвц и остальное вообще?) С одной стороны класс должен выполнять ровно одну задачу, и в это вписывается класс-парсер, с другой хорошо бы разделить их по другому - так как сделано в pdo, соединение создает объект-запрос и вертишь его как хочешь.

grigori · 26 Авг 2013

в enterprize environment, в котором я работаю, править непосредственно файлики библиотек, фреймворков и API просто невозможно

как бы ни хотелось иногда, если после правки слетят сайты - это жопа

но зафиксировать функционал по принципу "не нравится - не используйте" можно

grigori · 26 Авг 2013

hell0w0rd "при чем тут мвц" - это отсылка на тему многолетней давности о контроллерах, тебя тогда не было, наверное
"хорошо бы разделить их по другому" - предлагай

hell0w0rd · 26 Авг 2013

grigori
Так предложил же, объект-коннект, объекты-запросы. Коннект рождает запрос, ну чего я рассказываю - pdo, только без кривого интерфейса

По поводу проектов, где нельзя править - почему нельзя унаследоваться, и раз все через DI, тем более почему? Ровно как и форкнуть и поменять в композере ветку обновления, хотя второй вариант геморойнее

Breeze · 26 Авг 2013

grigori написал(а):
нет, я предлагаю сделать что-то типа

PHP:

interface iSafeDbParser{ } class SafeDb{ protected $parser; setParser(iSafeDbParser $P){$this->parser = $p;} getParser(){return $this->parser? : new SafeDbParser;} } $DB = new SafeDb; $DB->setParser(new MyParser);

ибо tell don't ask

ну на то и "псевдокодик" =) главное посыл был пойман правильно

Вурдалак · 26 Авг 2013

grigori написал(а):
чем? я считаю, что меньше параметров у конструктора - удобней API

Тем, что объект класса SafeDb не имеет смысла без инстанса iSafeDbParser. По определению, такие зависимости лучше передавать через конструктор.

Если так хочется хардкодить класс по умолчанию, то можно сделать аналогично в конструкторе:

PHP:

class SafeDb
{
	protected $parser;

	public function __construct(iSafeDbParser $parser = null)
	{
		$this->parser = $parser ?: new SafeDbParser;
	}
}

Тогда твой protected $parser всегда будет содержать инстанс объекта. Иначе возникает вопрос: если твой код предполагает обращение всегда через getter, то почему $parser protected, а не private? Говоря умными словами, тут налицо нарушение инкапсуляции для наследников класса.

grigori написал(а):
tell don't ask - концепция, по которой класс не должен запрашивать параметр у другого объекта, он должен получить его извне,
аналогично, не надо ждать, что в класс передадут значение в конструкторе, лучше позволять переопределить значение, и этим уменьшить связанность, упростить API

Не понял: каким боком тут «tell, don't ask»? Ты просто перенёс внедрение зависимости в setter, хотя правильнее через конструктор, ибо см. выше.

Ты сейчас, между прочим, хитришь: ты захардкодил какую-то конкретную реализацию парсера (для MySQL) и говоришь, что упростил API, уменьшил связность. Во-первых, теперь твой API неявно требует вызова setParser() вместо явного создания объекта с нужным инстансом парсера для Postgres, например, а, во-вторых, связность наоборот увеличилась из-за хардкода конкретной реализации. К тому же, ты добавил getter (я так понимаю public, да?) — это как раз провоцирует на нарушение озвученного тобой принципа. Ну и вообще: чем реализация для MySQL настолько лучше остальных, что она сделана по умолчанию? Ничем. Так вот если ты уберёшь хардкод, то получится, что вызывать setParser придётся всегда, появится размазывание API, конструктор будет изначально создавать объект в невалидном состоянии. Кстати, хотя передача зависимости через конструктор и не вступает с противоречием наличия метода setParser(), но смена парсера налету мне кажется надуманной идеей и как раз усложняющей API: нафиг надо?

А по поводу сложного конструктора — люди пользуются фабриками, DIC, вот этим всем.

AmdY · 26 Авг 2013

grigori
так вот именно, это не кровавый энтерпрайз, некоторым вещам стоит оставаться простыми и удобными.

А для энтерпрайзе иногда вынуждены использовать автопатчи. Всё равно куча тестов и либы редко обновляются.

grigori · 26 Авг 2013

Вурдалак написал(а):
SafeDb не имеет смысла без инстанса iSafeDbParser. По определению такие зависимости лучше передавать через конструктор.

это по какому или чьему определению? пруф plz, ссылку, если это больше, чем личное предпочтение

я помню множество форм DI/IoC, и Interface Injection - один из них,
еще могу gof процитировать

хардкодить класс по умолчанию можно аналогично в конструкторе:

новый холивар: setter injection VS constructor injection !

оставляю тебя победителем этой специальной олимпиады

налицо нарушение инкапсуляции для наследников класса.

да, это демонстация принципа, а не сам код, herr grammar nazzi

понятно, что во фреймворке все будет сложнее, с контейнером, точной доступа и блек джеком,
но в этой теме мы обсуждаем именованные плейсхолдеры, я DI просто мимоходом упомянул

Вурдалак · 26 Авг 2013

grigori написал(а):
это по какому или чьему определению? пруф plz, ссылку, если это больше, чем личное предпочтение

По определению конструктора. Во всяком случае, в моём понимании, конструктор должен создавать валидный объект: http://martinfowler.com/articles/injection.html#ConstructorVersusSetterInjection. Это подход правильнее, но не всегда удобнее. А уж в приведённом примере, очевидно, никаких неудобств нет.

То есть я не против того или иного: всему своё место. Если объект опциональный, то лучше как раз через setter.

Ты ж к чему-то тут приплёл «tell, don't ask». А к чему — так и не объяснил. А ведь так хотелось послушать учителя.

fixxxer · 26 Авг 2013

Строгий constructor injection, без создания "дефолтного" инстанса, требует, чтобы ВСЁ работало через DI, это "правильно", но на практике неудобно. В 90% случаев дефолтное поведение всех устраивает, и получаем просто кучу лишней писанины.

Если же инъекция зависимости опциональна, и при отсутствии оной создается дефолтный инстанс - нет никакой разницы, в конструкторе или через сеттер:

PHP:

class Foo {
    protected $dep=null;

    protected function getDep() {
         if (null === $this->dep) {
            $this->dep = new DefaultDep;
         }
         return $this->dep;
    }

    public function setDep(IDep $dep) {
        $this->dep = $dep;
    }
}

PHP:

class Foo {
    protected $dep=null;

    public function __construct(IDep $dep = null) {
        $dep and $this->dep = $dep;
    }

    protected function getDep() {
         if (null === $this->dep) {
            $this->dep = new DefaultDep;
         }
         return $this->dep;
    }

}

Не вижу вообще никакой разницы.

При чем тут tell don't ask, я тоже не понял.

Constructor injection VS Setter Injection - флейм из плейсхолдеров

goshogun

( ͡° ͜ʖ ͡°)

( ͡° ͜ʖ ͡°)

Продвинутый новичок

Пью пиво

Продвинутый новичок

( ͡° ͜ʖ ͡°)

( ͡° ͜ʖ ͡°)

( ͡° ͜ʖ ͡°)

Пью пиво

Продвинутый новичок

( ͡° ͜ʖ ͡°)

( ͡° ͜ʖ ͡°)

Продвинутый новичок

goshogun

Продвинутый новичок

Пью пиво

( ͡° ͜ʖ ͡°)

Продвинутый новичок

К.О.