Корректно ли реализована DI?

Question

[kot-samolet]

Вводные: класс реализующий методы работы с БД и класс которому нужна БД никак не связаны между собой (не наследники).
Цель:
1) что бы класс которому нужна БД мог пользоваться ей без передачи каждый раз ему БД в явном виде
2) Параметры подключения вынесены из класса БД.
Код:

class Config
{
    public $host = 'localhost';
    public $db =   'loadbot_db'; 
    public $charset = 'utf8';
    public $user = 'user';      
    public $pass = 'qwerty'; 
}

class DB
{
    public function __construct(Config $config) {
        // тут происходит подключение к БД
    }
    // тут все остальные методы работы с БД
}

class NeedDB
{
    public $db;
    
    public function __construct() {
        $cache = new Config();
        $this->db = new DB($cache);
    }
}

$var = new NeedDB();

Корректно ли реализовано, есть ли недостатки, можно ли улучшить?

Answer 1

[index0h]

1. Не используйте публичные свойства
2. Для описания параметров имеет смысл использовать константы

Comments

[kot-samolet]

Согласен, тут пока просто на это не заморачивался. Спасибо

Answer 2

[Иван Корюков]

В вашем коде нет DI.
Задача DI - избавиться от жёсткой связи между классами. Т.е. если вы заходите вместо класса DB в классе NeedDB использовать другой класс (ну там тесты гонять или задача такая), то вам придётся редактировать код класса NeedDB. DI - это техника, когда вы "снаружи" определяете какой класс будет использоваться.
Обычно для этого выделяется какой-нибудь ServiceLocator - объект у которого можно попросить объект нужного класса, и он отдаст его. Более радикальный способ - всегда передавать зависимости через конструктор. Этот способ идеологически более чистый, но и более навязчивый, т.к. вам надо всегда протаскивать зависимости через конструктор.

Comments

[kot-samolet]

Ок, можно сделать так:

public function __construct() {
        $db = new ServiceLocator();
    }

Тогда ServiceLocator вернёт уже нужный объект сразу.

"снаружи" определяете какой класс будет использоваться

Идея понятна, непонятно если у меня по этому классу будут разбросаны обращения к методам БД что толку если ServiceLocator вернёт мне объект другого класса? Мне нужен этот

вам надо всегда протаскивать зависимости через конструктор

Этого как раз стараюсь избежать

[Иван Корюков]

kot-samolet: нет не так, а вот так:

public function __construct(){
    $db = ServiceLocator::get("database");
}

В зависимости от того, какой объект вы засунете в ServiceLocator в bootstrap скрипте, такой объект и будет возвращаться. Например вы можете на боевом сайте засунуть туда класс MysqlDB, а для того чтобы прогонять тесты - засунуть туда MockDB.

непонятно если у меня по этому классу будут разбросаны обращения к методам БД что толку если ServiceLocator вернёт мне объект другого класса?

Ну так нужно засовывать класс нужного интерфейса. Зачем вам DI если вы не знаете зачем вам может понадобиться заменить один класс на другой?

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

Answer 3

[Максим Федоров]

1. Что-то Вы напутали, в Вашем примере нет DI. Принцип Dependency Injection говорит нам что классы не должны зависеть от реализации конкретных объектов, они должны зависеть от интерфейсов. В Вашем же случае интерфейсов нет, класс DB зависит от реализации класса Config - что само по себе являет нарушением принципа инверсии зависимости. По сути дела Ваш код сейчас - это реализация принципа Inversion of Control и не более..
2. Как сказал index0h не стоит использовать публичные свойства - эта плохая практика. Почему так не стоит делать - можно почитать в интернете, об этом написано не мало статей
3. Класс NeedDB как-то не понятно сделан. какое его назначение? создать объекты? тогда лучше использовать фабрику, либо Service Locator, либо Dependency Injection Container

Comments

[kot-samolet]

Понял, что ничего не понял.
1) Мне не очень понятно что значит зависеть от интерфейса, ведь интерфейс просто задаёт обязательства для класса реализовать методы указанные в нём.
2) Опустим паттерны проектирования на секунду. Разве класс БД в любом случае от конфига не будет зависеть? Только если конфиг внутри него прописать
3) NeedDB - это просто любой класс которому для работы нужна БД

[Максим Федоров]

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

interface ConfigInterface
{}

class Config implements ConfigInterface
{}

class DB 
{
    public function __construct(ConfigInterface $config)
    {}
}

Таким образом класс DB знает только о том что может класс Config, но вообще не зависит от его реализации
2. Будет зависит от конфига, но это ничего не меняет...
3. Тогда тут нарушение принципа Inversion Of Control - класс который зависит от DB его же и создает, вместо того что бы ожидать его передачи с наружи