Infinitiweb / code-conventions

MIT License
8 stars 3 forks source link

Удаляем требование суффикса у интерфейсов. #20

Closed Nex-Otaku closed 4 years ago

Nex-Otaku commented 5 years ago

1. Предлагаю убрать требование суффикса "Interface" для интерфейсов из конвенции.

2. Считаю, что этот суффикс только уродует и захламляет код. Повторение бесполезной информации в коде, затрудняет его чтение.

3. С суффиксом "Interface" усложняется рефакторинг.

Например, у нас есть компонент Cache, который используется в 500 классов системы через внедрение зависимостей. Подключается конкретный класс Cache.

Допустим, нам понадобилось реализовать новый вид кеширования. Был файловый кеш, а мы решили сделать ещё кеш в памяти, для быстрого запуска тестов.

Без конвенции, нам пришлось бы изменить только один файл - Cache. Заменив в нём ключевое слово "class" на "interface", и написав две реализации этого интерфейса, мы успешно решаем задачу.

С конвенцией, к этим действиям прибавляется переименование класса и соответственно изменение 500 связанных с ним классов.

4. Откуда взялось требование суффикса интерфейса в конвенции?

Я уверен, что такое требование перешло в конвенцию из невнимательного прочтения соглашения PSR: https://www.php-fig.org/bylaws/psr-naming-conventions/

Если приглядеться внимательнее, станет ясно, что этот стандарт описывает не стандарт для кодирования, а стандарт написания самих текстов PSR. То есть это стиль документации, а не стандарт кода.

Видимо, разработчика конвенции ввёл в заблуждение заголовок "PSR Naming Conventions", и стиль не глядя был скопирован в соглашения кода.

5. В списке актуальных стандартов, разумеется, этого нет. Ни один из действующих и ни один из устаревших стандартов PSR не предлагает использовать суффиксы для интерфейсов. См. https://www.php-fig.org/psr/

hello-omny commented 5 years ago

Удалять нельзя. Почему:

не логично, если не использовать суфикс будет именно путаница:

public function getCampaignArrayForSubscribeWithoutFeeds(Subscribe $subscribe, bool $checkVisited = true): ?array

тут интерфейс или класс?

Если класс то это конкретная реализация поведения и конкретная механика работы с экземпляром, а если интерфейс множество реализация работы с несколькими возможными экземплярами.

Именно суфикс дает понять поведение кода.

public function getCampaignArrayForSubscribeWithoutFeeds(SubscribeInterface $subscribe, bool $checkVisited = true): ?array

Ожидается работа с конкретным интерфейсом, а не конкретной реализацией.

sanchezzzhak commented 5 years ago

1 Даже Симфони3, Yii2, ZEND2-3 использует суфиксы в интерфейсе. Похоже там тоже дураки не поняли PSR (рекомендацию).

БАЦ https://symfony.com/doc/current/doctrine/resolve_target_entity.html БАЦ https://symfony.com/doc/current/service_container.html БАЦ https://github.com/yiisoft/yii2/blob/master/framework/data/DataProviderInterface.php БАЦ https://github.com/zendframework/zend-mvc/blob/master/src/ApplicationInterface.php

2 У нас есть класс Subscribe и нам надо добавить интерфейс Subscribe, по нашим канонам нам надо интерфейс добавить в use что приведет к ошибке. Cannot declare class Subscribe, because the name is already in use

Я за оставить Суфикс в интерфейсах. Жду новую иссу на тему давайте удалим суфикс у Controller (ну мне просто не удобно рефакторить).

Nex-Otaku commented 5 years ago

Удалять нельзя. Почему:

... Ожидается работа с конкретным интерфейсом, а не конкретной реализацией.

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

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

Если класс то это конкретная реализация поведения и конкретная механика работы с экземпляром, а если интерфейс множество реализация работы с несколькими возможными экземплярами.

Мы можем передать в этот метод в одном случае MegaSubscribe extends Subscribe и во втором случае SuperSubscribe extends Subscribe Оба класса наследуют класс Subscribe. Метод обязан корректно обработать как родительский, так и любой из дочерних классов. (LSP)

В чём тогда разница с интерфейсом?

Nex-Otaku commented 5 years ago

1 Даже Симфони3, Yii2, ZEND2-3 использует суфиксы в интерфейсе. Похоже там тоже дураки не поняли PSR (рекомендацию).

БАЦ https://symfony.com/doc/current/doctrine/resolve_target_entity.html БАЦ https://symfony.com/doc/current/service_container.html БАЦ https://github.com/yiisoft/yii2/blob/master/framework/data/DataProviderInterface.php БАЦ https://github.com/zendframework/zend-mvc/blob/master/src/ApplicationInterface.php

Стандарты стиля кода Symfony, Yii и т. д. касаются только кода самого фреймворка, а не приложений на этом фреймворке.

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

То, что проект, допустим, Dodopay сделан на Yii, не означает что в проекте Dodopay обязательно использовать все соглашения кода фреймворка Yii "один в один".

Мы имеем право устанавливать свои, более удобные правила. Эта конвенция и есть пример уточнённых правил.

Есть только один "отраслевой стандарт", к которому лучше всего следовать любой команде разработчиков. Это PSR. В нём подобных ограничений нет.

Жду новую иссу на тему давайте удалим суфикс у Controller (ну мне просто не удобно рефакторить).

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

2 У нас есть класс Subscribe и нам надо добавить интерфейс Subscribe, по нашим канонам нам надо интерфейс добавить в use что приведет к ошибке. Cannot declare class Subscribe, because the name is already in use

Если есть только одна реализация, то интерфейс обычно не нужен. Достаточно использовать класс. Если же нам нужен полиморфизм и мы делаем несколько реализаций интерфейса, то одним "Subscribe" уже не обойдёшься. А значит, это будет примерно так: ` interface Subscribe {}

class SingleSubscribe extends Subscribe {}

class RecurrentSubscribe extends Subscribe {} ` Никаких проблем с именами.

jukovsky commented 5 years ago
  1. Тоже не очень понял, зачем тебе понимать интерфейс перед тобой или класс во время написания кода? Инкапсуляция как бэ.
  2. Yii и Symfony - вот тут как раз надо понимать где интерфейс, где что. Ибо это сторонний код, который ты не можешь изменять непосредственно. Но ты должен понимать как, зачем и почему он работает. Ну и согласен с Леонидом - апеляция к авторитету в данном случае неуместна.

Поэтому предлагаю перейти к аргументации конкретными примерами. Леонид привел вполне реальный и понятный случай. Теперь хочется увидеть контрпример, если таковой существует.

hello-omny commented 5 years ago

Условие

SubscribeInterface — интерфейс подписки

Subscribe implements SubscribeInterface
ReserveSubscribe implements SubscribeInterface

Subscribe::tableName() -> subscribe
ReserveSubscribe::tableName() -> reserve_subscribe

Subscribe::getDb() -> объект db
ReserveSubscribe::getDb() -> объект reserveDb

Главный вопрос: «Что говорит разработчику декларация первой, второй и третьей функции?»

  1. Что обработает и вернет следующий код?
  2. Какой метод какие исключения выбросит?
  3. Где можно применить?
  4. Как это заменить?
function save (SubscribeInterface $subscribe): SubscribeInterface 
{
    $subscribe->save();
    return $subscribe;
}
function save (Subscribe $subscribe): Subscribe 
{
    $subscribe->save();
    return $subscribe;
}
function save (ReserveSubscribe $subscribe): ReserveSubscribe 
{
    $subscribe->save();
    return $subscribe;
}
Nex-Otaku commented 5 years ago

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

Кстати говоря, "Subscribe" это глагол "подписаться", а здесь видимо имелось в виду существительное "Subscription", то есть "подписка". Лучше использовать существительные в названиях классов, так будет меньше путаницы.

hello-omny commented 5 years ago

@Nex-Otaku, для понимания я задал наводящие вопросы, которые тебе помогут понять и разобраться почему суффикс нужен, это одно из опровержений твоих замечаний:

  1. Считаю, что этот суффикс только уродует и захламляет код. Повторение бесполезной информации в коде, затрудняет его чтение.

  2. С суффиксом "Interface" усложняется рефакторинг.

Nex-Otaku commented 5 years ago

Не понимаю, каким образом этот код что-то опровергает.

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

С суффиксом итоговый код метода плюс определения выглядит так:

interface SubscriptionInterface {}

class Subscription implements SubscriptionInterface {}

class ReservedSubscription implements SubscriptionInterface {}

function save (SubscriptionInterface $subscription): SubscriptionInterface 
{
    $subscription->save();
    return $subscription;
}

Без суффикса будет так:

interface Subscription {}

class SimpleSubscription implements Subscription {}

class ReservedSubscription implements Subscription {}

function save (Subscription $subscription): Subscription 
{
    $subscription->save();
    return $subscription;
}

С использованием суффикса, слово Interface повторяется 6 раз, без суффикса всего 1 раз. По мне, второй код гораздо проще читается.

Если же интерфейс используется только для обработки в методе "save", то его стоит переименовать согласно цели:

interface Saveable {}

class SimpleSubscription implements Saveable {}

class ReservedSubscription implements Saveable {}

function save (Saveable $saveable): Saveable
{
    $saveable->save();
    return $saveable;
}
hello-omny commented 5 years ago

@Nex-Otaku, тогда скажи мне в коде ниже, Subscription это что? Код написан под Yii2, в наших проектах.

<?php

class SystemService
{
    function create (Subscription $subscription)
    {
        $subscription->save();
    }
}
jukovsky commented 5 years ago
  1. А какая мне разница что это?
  2. Ide мне подскажет
hello-omny commented 5 years ago
  1. Тебе может и не важно, а ТС топит за удобство — удобно когда четко видно, без лишних телодвижений что перед тобой. В предложенном варианте — не видно.
  2. Только при наведении или переходе в файл. Не всегда под рукой айдиешечка.