Pull to refresh

Comments 33

Так и в чем же принцип не работает? Вы же таки декомпозировали код, а не решили оставить как есть.

Ну и кроме того, сам SRP изначально как раз и формулируется в терминах связности и причин к изменениям: "There should never be more than one reason for a class to change". А вовсе не в терминах "делать что-то одно и хорошо". Это лишь следствие, которое лучше укрепилось в массовом сознании.

Да, все так. Статья в большей степени про то, что наивное восприятие принципа SRP приводит к проблемам, особенно у молодого поколения. Так-то сам принцип по разному формулировался Мартином в разное время и начинался с относительного "наивного" определения и пришел по итогу к по сути к принципу loose coupling & high cohesion. И LCHC на мой личный взгляд является более объемлющим и более применимым в разработке на самом разных уровнях.

Я считаю что в идеале код должен быть разделен на минимальные частицы, насколько это возможно.

Но мы можем "схитрить" и упростить код - взять на себя ответственность и объединить некоторые участки вместе, как это сделано в варианте 1 - get_post_for_user_id, заранее предполагая, что этот функционал тоже будет меняться вместе. Т.е. мы упрощаем архитектуру - ради повышения удобочитаемости и когнитивного восприятия кода, под собственное видение программиста каким образом бизнес будет развивать свой проект и с какими задачами на изменение бизнес будет приходить к нам (с какими "осями изменений", "линиями разреза" бизнес будет приходить к нам).

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

Первый вариант приводит к созданию 100500 копипащенных функций, каждая из которых используется лишь в одном месте. Ориентироваться в них будет сложно. Поддерживать - тем более. А рефакторинг вообще будет адом. В лучшем случае их содержимое будет тем же, что вы написали во втором варианте. Так что лучше посто перенести их содержимое в место их вызова. И не плодить сущности сверх меры. В одной большой функции разбитой на разделы комментариями разобраться проще, чем в куче функций, передающих друг другу управление. А вот выносить лучше то, что может быть переиспользовано в других местах, как у вас написанов во втором варианте. И, конечно, никаких "бесконечно много вариантов" тут нет. Разумные варианты разбиения обычно по пальцам одной руки можно пересчитать.

Первый вариант приводит к созданию 100500 копипащенных функций, каждая из которых используется лишь в одном месте.

Для некоторых языков (C#, Java, Delphi, С++ и прочих «объектно-ориентированных») есть ещё одна альтернатива — создание иерархии классов: общая функциональность выносится в базовый класс в виде базовой реализации метода (обычно виртуального), а специфическая — в специфичную для класса реализацию этого метода, либо вызывамый из него служебный виртуальный метод (см. Шаблон Template method). К примеру, у меня в моей недавней статье на Хабре, посвященной решению тестового задания (судя по рейтингу статьи — уже давно никому не интересного, а потому ссылку давать даже не буду) именно так и сделано на C# немалое количество мест, где нужно делать что-то похожее, но таки отличающееся.
Но вот как такое сделать в вашем основном рабочем (если я себе правильно представляю) языке JS — это я без понятия: я JS знаю весьма ограниченно.
Но вот как такое сделать в вашем основном рабочем (если я себе правильно представляю) языке JS — это я без понятия: я JS знаю весьма ограниченно.

Точно так же. Там такое же ООП как и в других языках, если не ломать его намеренно и не играть в ФП ради ФП.

Ну все-таки не совсем "точно такое же". Прототипы там, extends - это expression и справа можно вызов функции поставить. Вот это вот все

Но это весьма спорные возможности:) не просто так же все новые версии es и typescript делают из JavaScript подобие Java/C#

Какая разница, спорные они или нет? Ничто не мешает вам, при желании, просто игнорировать эти возможности и писать привычное ООП.

Ну все-таки начиная с появления ключевого слова class это "почти привычное ООП". Конструкторы ведут себя иначе, это нужно учитывать (потому что прототипы), static-члены весьма спорные, ибо можно просто сделать export func из модуля (как, кстати, обычно и делают). bind'ы тоже останутся много где. Так что same-same but different.

Вы путаете парадигму, её реализацию и синтаксис. Так-то на JavaScript можно было писать по всем канонам ООП даже до появления, собственно, классов. А уж после появления классов это стало делать точно не сложнее.


Конструкторы ведут себя иначе, это нужно учитывать

Как именно иначе они себя ведут и зачем это учитывать?

Удивительно читать такие слова от вас. Как скоро вы примените этот принцип к своему личному фреймворку?

Такое ощущение, что вы знаете про этот фреймворк больше меня самого.

Я знаю про него ровно то, чем вы поделились на Хабре, а вы выдаёте информацию весьма дозировано.


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

Никогда такого не было. Это как раз в Реакте люди страдают созданием отдельных компонент по каждому чиху, чтобы уменьшить ререндеры.

И где же там написано про создание отдельных компонент?

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

> Но точно помню, что ещё раньше вы кому-то доказывали в комментариях что циклы в шаблонах не нужны.

Конкретно в $mol, нет в "шаблонах" циклов и любой логики.

В привычном понимании шаблон - это что-то похожее на html с вкраплениями каких-то команд(инетрполяция,условия,циклы), служащее для того, чтобы обозначить структуру дом-поддерева/компонент и вложить в него/них данные(и еще что-нибудь, и отколенения в какую-то сторону - у всех свои вариации)

В $mol компонент - это ts-класс, описывается тремя файлами: "шаблон" без логики(view.tree), логика/поведение(view.ts), стилизация(view.css/view.css.ts)
Файлы с поведением/стлизацией опциональны.

Сам "шаблон", больше похож не на шаблон в привычном понимании, а на интерфейс - .т.е. он описывает каким будет класс компонента. А фреймворк из него генерирует ts-класс(view.tree.ts). А пользовтель создает файл с логикой(view.ts) в котором наследуется от сгенерировано класса и добавляет туда поведение

>Я знаю про него ровно то, чем вы поделились на Хабре, а вы выдаёте информацию весьма дозировано.
хотел написать про циклы, а получилась вводная про компоненты) Если интересно могу подробнее.

В одной большой функции разбитой на разделы комментариями...

Читая различные книги, различных авторитетных авторов (например того же дядюшки Боба "Чистый код") встречался с тем что необходимо избегать обильного комментирования кода. Если есть желание кусок кода "обернуть" комментарием то этот код самый верный кандидат на вынесение в отдельный метод или класс. Имя метода (класса) станет абстракцией/черным ящиком которая явно скажет что делает этот кусок кода.

Иначе код будет:

  1. Захламлен обильными комментариями, что затрудняет чтение кода, к тому же хорошие комментарии тоже надо уметь писать.

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

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

  4. Методы/функции будут непомерно большими и сложно понимаемыми.

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

Автор добавил комментарии, чтобы читатель быстро вник в контекст. Обычная практика при написании статьи. Да и функция большая делающая много чего, что комментарии напрашиваются.

куче функций, передающих друг другу управление

Обычно все таки не нужно знать как метод достигает цели, мы смотрим на него как на черный ящик. Если все таки нужно знать реализацию, то опять же имя метода говорит, что должен сделать метод, код расскажет как он это делает. Опять вижу только удобство;

Первый вариант приводит к созданию 100500 копипащенных функций

обычно когда делишь на меньшие методы/классы/... то больше шансов, что не будет копипаста. Вопрос только в том как правильно декомпозировать. Но похоже мы вернулись к теме поднятой автором).

  1. Хорошее имя функции придумать ещё сложнее хорошего комментария.

  2. Имена функций точно так же может разъезжаться с кодом.

  3. Лучше, если кода будет меньше, чем больше.

  4. Линейный код понимается легче, чем нелинейный.

  5. Комментарий тоже это рассказывает и позволяет перескакивать неинтересные куски.

  6. В контрасте нет ничего плохого.

Хорошее имя функции придумать ещё сложнее хорошего кокомментария

Если функция делает что-то одно, и программист знает, что именно она делает - не так уж и сложно.

Имена функций точно так же может разъезжаться с кодом

... Но заметить это проще. Сто раз ловил себя на том, что при анализе кода комментарии автоматически фильтруютя и вообще не воспринимаются как смысловая часть.

Лучше, если кода будет меньше, чем больше.

Линейный код понимается легче, чем нелинейный

Согласен. Но удачная декомпозиция позволяет вообще не лезть в детали. Гораздо проще увидеть общую картину. Даже если кода в целом больше, то в фокусе внимания намного меньше, и на каждом уровне обобщения он связан с чем-то одним. И получается более линейным — редко где глубокие вложения остаются.

Комментарий тоже это рассказывает и позволяет перескакивать неинтересные куски.

Даже не заглядывать в неинтересные куски, не искать конец неинтересного куска (потому, что он представлен одной строкой вызова) — всё это признаки удачной декомпозиции.

В контрасте нет ничего плохого.

Хорошее вообще встречается чаще плохого, а правильные решения чаще неправильных (при приложении осознанных и целенаправленных усилий). Главное не доводить до маразма следование любым теориям, и уважать и ценить чужой труд. И любой код перестанет огорчать. Даже тот, который написал я-вчерашний.

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

А по моему в данном виде не нужно вести рассуждения об единственной ответственности. В данном случае не определена задача. Но всё же порассуждаем. Логически если есть проверка на владельца поста, значит их много. А если много пользователей, то есть администратор/модератор/редактор. Следовательно есть другой код для них и значит единственная ответственность правка поста размазана по нескольким местам)))

Другими словами нам нужно вести речь не о получении пользователя и связанного с ним поста, а с получением редактирующего, поста, проверкой прав на правку данного поста редактирующим и так далее.

Написав метод get_post_for_user_id мы потеряли информацию.

Во-первых метод должен называться get_post_by_post_id_and_user_id. Потому что название get_post_for_user_id больше подходит для get_posts_for_user_id - когда мы достаем все посты по user_id. Еще есть за что зацепится по П.1: у нас в базе нет одной сущности post для user_id, у нас есть массив posts для user_id. Т.е. в базе есть только однозначное соответствие массива постов posts и user_id. И нет однозначного соответствия какого-то одного поста для user_id. Но может быть last_post_for_user_id, most_rated_post_for_user_id, random_post_for_user_id и т.д. В названии get_post_for_user_id теряется однозначность - какой пост мы достаем = теряется информация.

Во-вторых при применении этого метода в проверке прав доступа, при отсутствии поста (пост с post_id не существует) должна быть одна ошибка, а если post.user_id != user_id, то это уже другая ошибка. Если мы и там и там создаем одну и ту же ошибку - информация теряется. Даже если мы наружу отправляем и там и там одну - 404 ошибку, внутренняя ошибка должна быть разная (для логов, для security отдела и т.д.).

Серьёзно, в названии метода надо дублировать используемые в нем параметры? Я бы назвал функцию post_for_user()

Нет единственно верного разделения или выделения ответственностей для классов, функций, методов, модулей, пакетов и т. д.

Единственного верного нет, но есть предельный случай, до которого, например, доходят в книжке Growing Object-Oriented Software, Guided by Tests (GOOS).

Они разделяют классы/методы до упора, пока они делятся по зонам ответственности. Пользователя, и права пользователя они бы точно разделили.

Я не говорю, что это хорошо, просто слово "единственный" напомнило эту книжку.

Не силен в Питоне, но кажется здесь в примере кода ошибка:

def get_post_by_id(db, user_id): 
    post = db.execute(
        'SELECT p.id, title, body, created, author_id, username'
        ' FROM post p'
        ' WHERE p.id = ?' ,
        (post_id,)
    ).fetchone()

Должно быть так:

def get_posts_by_user_id(db, user_id): 
    post = db.execute(
        'SELECT p.id, title, body, created, author_id, username'
        ' FROM post p'
        ' WHERE p.author_id = ?' ,
        (user_id,)
    ).fetchone()

или так:

def get_post_by_id(db, post_id): 
    post = db.execute(
        'SELECT p.id, title, body, created, author_id, username'
        ' FROM post p'
        ' WHERE p.id = ?' ,
        (post_id,)
    ).fetchone()

Читая пост у меня возник вопрос:

Почему автор использует структурное программирование и рассуждает на тему SRP из набора принципов SOLID?

В то время как SOLID это набор принципов предназначенных для объектно ориентированного программирования.

Как мне кажется автор перепутал парадигмы программирования и принципы разбиения программ. Ведь в структурном программировании нет принципов SOLID, но есть другие принципы посторения программ.

Конкретно этот принцип подходит и для структурного программирования, и для функционального, и для любых других. И в ООП у SRP ровно такие же проблемы, как и из примеров в статье, т.е. можно было бы вынести код не в функцию, а в классы и было бы то же самое.

Во первых, этот принцип не подходит, из определения принципа следует что он применяется к ООП.

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

Но если бы мы работали в парадигме ООП то мы бы шли другим путем:

  1. Сразу обозначим что у нас не только создание поста пользователя а есть некая систем (в противном случае ООП здесь не имеет никакого смысла)

  2. Выделим операцию UpdatePost это интерфейс который будет заниматься изменением поста на основе данных в data transfer object (dto) UpdateRequest

interface UpdateRequest {
  invoke(request: UpdateRequest)
}

data class UpdateRequest(
	private val title,
  private val authorId,
  private val body
)
  1. Реализуем основную логику создания поста

class CommonUpdatePost(
	private val db
) : UpdateRequest {
	fun invoke(request: UpdateRequest) {
    db.execute("...", ...): // тут сам запрос и параметры запроса
  }
}
  1. Реализуем класс отвечающий за ограничение прав доступа, стоит отметить что получение текущего пользователя лучше вынести в отдельный сервис, т.к высока вероятность что это потребуется в нескольких операциях

class AccessCheckerDecorator(
	private val currentUserProvider, 
  private val inner
) : UpdateRequest {
	fun invoke(request: UpdateRequest) {
    if(currentUserProvider.provide().id != request.authorId) {
      throw new AccessDeniedException("You cannot change post author");
    }
    inner.invoke();
  }
} 
  1. Реализуем класс который отвечает за определенные бизнес правила

class BusinessRuleCheckerDecorator(
	private val db,
  private val inner
) : UpdateRequest {
	fun invoke(request: UpdateRequest) {
    if(isPostExists(request.id)) {
      throw new NotFoundException("Post not found with id ${request.id}");
    }
    inner.invoke();
  }
  
  private fun isPostExists(Int postId): Boolean {
    // здесь собственно запрос
  }
} 
  1. Давайте разберемся с валидацией. Первое что нужно сделать понять как мы будем валидировать, например для этого есть инструменты позволящие навешать аннотации и отправить некий сервис, тем самым вообще упростив нашу логику и делать это в контроллере, но можно и тут добавить еще один декоратор. по аналогии с предыдущими проверками

  2. Рендеринг. В моем примере нет возвращаемого результата, поэтому в принципе мы можем и не возвращать его а использовать запрос т.к все данные будут идентичны, и рендеринг это ответственность контроллера.

  3. По идеи сам контроллер будет сведен к набору действий:

    1. Вытащить данные из источника дыных (будь то stdin или некие объекты хранящие данные HTTP запроса)

    2. Десериализация строки в dto объект

    3. Валидация dto объекта

    4. Вызов операции

    5. Рендеринг (или сериализация dto в некую структуру)

Что это разбиение дает?

  1. Мы изолировали ответственности и теперь расширение/изменение отдельных кусков операции мы можем делать независимо, тем самым минимизировав риски конфликтов в репозитории (например изменение логики проверки доступов)

  2. Спорно (т.к на практике не все работают в этой парадигме, хотя считают что работают), но если работать в такой парадигме, то очень легко понимать что происходит.

  3. В случае появления определенных повторений, которые можно незаметить в другой парадигме, можно выделять в общие сервисы.

  4. Меньше состояний и их мутации это дает нам лучше контроллировать программу

  5. Очень много файлов между которыми нужно переключаться

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

  7. Требует знания и навыков применения не только SRP но и других принципов, а так же знания паттернов ООП.

  8. При большом числе реализаций операции, очень сложно вносить изменения в операцию, зачастую это приводит к созданию новой операции почти с идентичной логикой, что бы не нарушать согласованность и другие зависимости (например от API).

В заключении хочу сказать, SRP действительно иногда бессмысленен, но это зависит от объема проекта и задачи, в некоторых случаях действительно он не нужен да даже и ООП, но к сожалению не в том контексте в котором вы продемонстрировали (обновление некого поста, что как бы намикает на то что уже у нас есть создание/удаление/получение списка или данных по id а так же работа с авторами постов, т.е пользователями).

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

Sign up to leave a comment.