За последние 24 часа нас посетили 18983 программиста и 1609 роботов. Сейчас ищут 1047 программистов ...

PHP Value Objects - оцените библиотеку

Тема в разделе "Решения, алгоритмы", создана пользователем myks92, 12 мар 2020.

Метки:
  1. myks92

    myks92 Новичок

    С нами с:
    12 июн 2018
    Сообщения:
    45
    Симпатии:
    1
    Всем Привет! Создал библиотеку для работы с популярными Value Objects. Библиотеку планирую оставить на Open Source. Буду её поддерживать. Так же готовлю паралельно пакет для использования в doctrine. Сейчас готовлюсь к выпуску первой версии. Постарался максимально подготовить пакет для релиза. Покрытие тестами сейчас 100%.

    Однако хотелось бы услышать от Вас немного критики по этой библиотеке. Подскажите, что тут по-вашему мнению можно изменить/усовершенствовать/дополнить. Лично у меня сейчас возникло несколько таких нюансов:

    1. Имеет ли смысл каждый VO дополнить своим интерфейсом, чтобы в коде можно было использовать не сами объекты, а абстракцию. Конечно, все объекты используют ValueObjectInterface, но может быть этого не достаточно?

    2. Имеет ли смысл в Birthday в конструктор передавать не DateTimeImmitable, а просто значение. В самом конструкторе это значение помещать в DateTimeImmitable и присваивать Value.

    3. У меня есть отдельная группа Option для всех опциональных типов. Они все наследуются от Enum, который лежит в своей папке. Нужно ли его переместить в Option или лучше оставить в своей папке? Так же и вопрос по Payment и Money. Может быть их тоже объединить? Хотя, мне кажется, это разное.
     
  2. artoodetoo

    artoodetoo Суперстар
    Команда форума Модератор

    С нами с:
    11 июн 2010
    Сообщения:
    11.105
    Симпатии:
    1.243
    Адрес:
    там-сям
    @myks92 Как насчёт примеров использования?
     
  3. ElisDN

    ElisDN Активный пользователь

    С нами с:
    13 фев 2018
    Сообщения:
    605
    Симпатии:
    130
    1. Не имеет. И ValueObjectInterface не нужен.

    2. Не имеет. Это не его забота.

    3. Option у вас сейчас наследуется от StringLiteral.
     
  4. myks92

    myks92 Новичок

    С нами с:
    12 июн 2018
    Сообщения:
    45
    Симпатии:
    1
    1. А почему не нужен? Если пользователь захочет дополнить свой функционал другими VO? Можно просто реализовать общий интерфейс. Или же лучше отказаться от интерфейса в пользу абстрактных классов вроде Enum, от которых наследоваться?

    3. Да, я ошибся немного. Он наследуется от StringLiteral потому что не имеет никаких констант. Наследование этого класса от Enum не будет иметь смысла для финальных классов. А вот имеет ли смысл для Status и Roles сделать простой класс, который уже можно будет наследовать, а в библиотеке поменять наследование с StringLiteral на Enum? Какие ещё VO лучше сделать абстрактными или же простыми классами, чтобы в коде можно было создавать свои?

    --- Добавлено ---
    Примеры использования есть в документации: https://github.com/Myks92/php-value-objects/blob/master/docs/readme.md
     
  5. artoodetoo

    artoodetoo Суперстар
    Команда форума Модератор

    С нами с:
    11 июн 2010
    Сообщения:
    11.105
    Симпатии:
    1.243
    Адрес:
    там-сям
    Это прекрасно. Но если ты хочешь фидбека здесь, то короткое введение не помешало бы.

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

    По поводу имутабельности: она не является обязательным требованием VO, хотя многие считают, что это было бы полезно. Уж точно нельзя требовать инициировать объект-значение неизменяемыми данными (DateTimeImmutable).
    --- Добавлено ---
    Интерфейсы нафиг не нужны здесь.
    --- Добавлено ---
    Payment не может быть VO по определению. Так как платёж должен быть идентифицируемым.
     
  6. ElisDN

    ElisDN Активный пользователь

    С нами с:
    13 фев 2018
    Сообщения:
    605
    Симпатии:
    130
    В общем случае простые VO достаточно редко имеет смысл выносить в отдельный пакет. VO вроде этого:

    PHP:
    1. class Email
    2. {
    3.     private string $value;
    4.  
    5.     public function __construct(string $value) {
    6.         Assert::email($value);
    7.         $this->value = mb_strtolower($value);
    8.     }
    9.  
    10.     public function isEqualTo(self $other): bool {
    11.         return $this->value === $other->value;
    12.     }
    13.  
    14.     public function getValue(): string {
    15.         return $this->value;
    16.     }
    17. }
    из десяти строк слишком просты, чтобы ради них подключать целую библиотеку. И наоборот, если пользователь захочет дополнить VO из библиотеки, то ему будет проще скопировать класс из библиотеки к себе и прямо в свой класс дописать нужный метод. Либо просто отнаследуется от вашего VO и добавит свой метод, но так как вы ваши классы объявили как final это у него сделать не получится. И интерфейсы для каждого класса пользы не принесут, так как скопипастить простой класс проще, чем реализовывать интерфейсы.

    А общий интерфейс ValueObjectInterface вообще нигде никем использоваться не будет. Если он нужен лишь для описания метода __toString(), то тоже пользы мало, так как встроенный метод __toString() вполне можно использовать и без интерфейса. И если доменные сущности и VO не используют для рендеринга, то этот метод будет лишним.

    Полезное - это убрать Status и Role и оставить только абстрактный класс Enum, по которому пользователь сможет отнаследовавшись сделать свой класс Status или Role со своими константами.
     
  7. myks92

    myks92 Новичок

    С нами с:
    12 июн 2018
    Сообщения:
    45
    Симпатии:
    1
    Вы говорите, что Email слишком просты, чтобы выносить в пакет. А если Email используется ни в одном месте? Тоже лучше просто копировать?

    В целом я понял. Получается и правда в этой библиотеке нет особого смысла даже если у классов убрать final? Думал, что это как-то облегчит использование в своих проектах, чтобы не заниматься копипастой кода и тестов, а так же поможет другим. Да и подключение к доктрине занимало бы меньше времени. Так как планировал сделать Doctrinе Types, которые подключаются в DI очень просто.

    Тогда у этой библиотеки получается два варианта будущего:
    1. Оставить всё так и использовать для копипасты.
    2. Убрать в классах final и использовать наследование в своих проектах.

    Как лучше поступить? Я думаю, что склоняетесь к первому варианту.
    --- Добавлено ---
    По обсуждениям я уже понял, что создал лишних классов, которые в принципе не очень нужны. Об этом я не подумал. Получается сами по себе VO в проекте нужны только с использованием расширения, а не готовых типов. Но это относится не ко всем классам.

    По иммутабельности: вы считаете лучше использовать строку?
     
  8. artoodetoo

    artoodetoo Суперстар
    Команда форума Модератор

    С нами с:
    11 июн 2010
    Сообщения:
    11.105
    Симпатии:
    1.243
    Адрес:
    там-сям
    строку? я комментировал вот это
    DateTimeImmutable - вот это лишнее. можно наверное указать тип DateTimeInterface чтобы допускать разные варианты
    --- Добавлено ---
    или mixed и допускать int | string | DateTimeInterface
     
    myks92 нравится это.
  9. myks92

    myks92 Новичок

    С нами с:
    12 июн 2018
    Сообщения:
    45
    Симпатии:
    1
    Понял!) Так и сделаю
     
  10. ElisDN

    ElisDN Активный пользователь

    С нами с:
    13 фев 2018
    Сообщения:
    605
    Симпатии:
    130
    В своих проектах использовать свои же VO удобно. Но с чужими сразу же обнаружатся неудобства:

    - Ваш класс Gender содержит всего три константы, а у Facebook в выпадающем списке их целых 54.
    - Кому-то в Email нужен strtolower, а кому-то не нужен, так как нужно сохранять оригинал.
    - В вашем Network есть только isEqualTo, а у кого-то ещё может понадобиться и ifForNetwork.
    - Ваш Money хранит amount в int, но не имеет множителя, нужного для хранения копеек или центов.

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

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

    myks92 Новичок

    С нами с:
    12 июн 2018
    Сообщения:
    45
    Симпатии:
    1
    Спасибо за толковое разъяснение) Тогда почищу библиотеку от лишнего, уберу final и оставлю. Изначально делал без final. А вот про Facebook с 54 новых пола удивили))
     
  12. myks92

    myks92 Новичок

    С нами с:
    12 июн 2018
    Сообщения:
    45
    Симпатии:
    1
    Добил библиотеку! Всем спасибо) Первый релиз готов. В "бою" покажет))
     
    artoodetoo нравится это.