LINUX.ORG.RU

ООП - SRP, есть упоротость.

 


0

1

Есть класс книги.
Вот варианты использования:

  • Можно получить контент книги для прочтения ее.
  • При редактировании контента, книга сохраняется в БД.
  • Книгу можно получить только по заранее известному id (неважно откуда юзер узнает)
  • Можно распечатать на принтере. Любую книгу можно печатать.

Что заранее известно:
Никогда в жизни не будет нескольких хранилищ книг, и оно никогда не изменится.
Вот код:

class Book {
    private int id;
    private String content;
    private Database database;
    private Printer printer;

    public Book(int id) {
        database = new Database("table-books");
        printer = new Printer();
        this.content = database.select(id);
    }

    public String getContent() {
        return this.content;
    }
    
    public void setContent(String content) {
        this.content = content;
        save();
    }

    public void print() {
        // предположим, что тут сложная логика из 15 строк для работы с принтером. Но больше ни один класс печататься не умеет
        printer.selectPrinterAndPrint(getContent());
    }

    public void save() {
        // предположим тут сложная логика сохранения в бд, но больше ни один класс не сохраняется
        database.updateOrInsert(id, getContent());
    }
}

Но как гласит принцип, у класса должна быть только одна причина для изменения.
Получается, что любой чувак из интернетов, кто пишет умные статьи, меня с таким кодом пошлет куда подальше.
Какого черта мне здесь создавать отдельные классы Storage и PrintingSystem ради инкапсуляции логики туда? Если никто больше юзать не собирается их. Какого класс, который юзает Book я должен еще утяжелить знаниями о Storage и PrintingSystem?
Мне кажется, что все эти принципы должны учить только одному - здравому смыслу.

Перемещено leave из talks

чтоб потом не было мучительно больно. Очевидно же. Но для хеловорда «и так сойдет»

поставь тег «тупняк анонiмуса», уж больно на его теоретику похожи изыскания.

anonymous ()
Ответ на: комментарий от anonymous

Какая то извращенная логика. Должно быть хранилише для книг, именно оно должно получать id книги и выдавать ее.

callbackhell ()
Ответ на: комментарий от anonymous

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

Че то не понятно, а где ты класс то меняешь?

callbackhell ()

Если никто больше юзать не собирается их.

Количество пользователей класса неважно. А что важно так это разбитие программы на изолированные классы с well defined behavior. Естественно чем мельче класс, тем проще определить его поведение. Раздутие API одного класса сторонним функционалом ведёт нарушает этот принцип, ведь теперь нужно определять поведение для каждого вызова этого API в каждом из состояний объекта, количество вариантов растёт в экспоненциальной прогрессии. Вобщем, читать в сторону RAII.

Dendy ★★★★★ ()

Мне кажется, что все эти принципы должны учить только одному - здравому смыслу.

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

callbackhell ()

Либо, уж если в таком говностиле, самоцель — обойтись без хранилища, то я бы реализовал такой интерфейс

theBook = Book with(id)
theBook read
theBoor setContent(...)

callbackhell ()
Ответ на: комментарий от anonymous

Посмотрел https://github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition. Там жеж процедурщина на классах. Много классов имеющих только одну функцию - действие, не имеющие состояние. Я раньше так писал пока в меня коллеги не стали кидать ссаными тряпками.

pozitiffcat ★★★ ()

Убрать из книги базу данных и принтер. Это внешние объекты которые должны работать с книгой (получать её как параметр) получается что у нас осталось только айди и стринг. Начинаем думать что такое аиди, и зачем оно в книге, может всётаки этот айди не является книгой? может это всётаки какойто атрибут который может хранится гденибудь снаружи? типа списка книг. тоже убираем его. Останется простой стринг. он то и есть твоя книга. не нужен вообще этот класс книга.

CyberK ()
Ответ на: комментарий от pozitiffcat

Ниче ты в интерпрайзе не понимаешь.

anonymous ()
Ответ на: комментарий от pozitiffcat

Много классов имеющих только одну функцию - действие, не имеющие состояние

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

Я раньше так писал пока в меня коллеги не стали кидать ссаными тряпками.

А ты свое мнение стараешься не отстаивать по-жизни? Тебе на него насрать, лишь бы посанов все устраивало?

callbackhell ()
Ответ на: комментарий от callbackhell

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

pozitiffcat ★★★ ()
Ответ на: комментарий от pozitiffcat

Солид — это говно, оно к настоящему ООП отношения не имеет. Нормально писать, ящетаю, это когда код логичен, синтаксически сладок, эргономичен, по максимуму использует делегирование, не ограничивает манипуляции метаобъектами и экстремально расширяем и гибок. Вот это основные критерии для меня. У вас в ынтырпрайзе свои идиомы конечно. При «совместной разработке» о нормальном ООП можно забыть, поскольку и языки и технологии рассчитаны на среднестатистическую серую мышь. Поэтому смоллтоклайк языки и здохли.

callbackhell ()
Ответ на: комментарий от callbackhell

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

pozitiffcat ★★★ ()
Ответ на: комментарий от pozitiffcat

Я собственно не говорил, что надо разбивать без нужды, просто ради разбиения. Просто когда начинаешь писать, ты уже можешь сразу предугадать, что придется расширять впоследствии, что нет, вот этим и руководствоваться надо.

callbackhell ()
Ответ на: комментарий от pozitiffcat

Но я уже говорил, у тебя страдает логичность. Я еще готов смирится с тем, что книга сама себя печатает, но у тебя книга получает сама себя по айди, это никуда не годно:).

callbackhell ()
Ответ на: комментарий от pozitiffcat

ты этого кукаретика слушай больше. Он же ни одного проекта сложнее хеловорда не написал. из него проектант, как из говна пуля.

anonymous ()
Ответ на: комментарий от pozitiffcat

Вообще, это все таки, несколько жабаспецифичная проблема. В более других языках, ты мог бы, например, создать хранилище как экземпляр Object, то есть, обычный объект. Соответственно, дилема расширяемость/нерасширяемость вообще бы не возникла. У тебя был бы просто объект storage из которого ты б доставал книги, а если бы потом появилась необходимость сделать из него класс, ты бы с легкостью переписал код не трогая остальное.

callbackhell ()
    public void setContent(String content) {
        this.content = content;
        save();
    }

Пованивает. А как ты тестировать собрался эти сложные другие логики? Через Book?

A1 ()
Ответ на: комментарий от anonymous

ты этого кукаретика слушай больше.

Внезапно этот кукаретик говорит дельную вещь. Лезть в констукторе в базу это тотальная упячка.

A1 ()
Ответ на: комментарий от A1

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

pozitiffcat ★★★ ()
Ответ на: комментарий от pozitiffcat

Не юнит, но тестится.

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

A1 ()
Ответ на: комментарий от pozitiffcat

Конкретно в этом примере нет ни одной причины нарушать SRP. Тем более это жабка, все нужные интерфейсы генерит IDE. То есть даже лень как аргумент не канает.

A1 ()

Еще пример:

class RequirementCalculator {
    public int calculateRequirement(Day day, User user) {
        // some logic
    }

    public int calculateUsed(Day day) {
        // some logic
    }
}
Этот класс считает норму потребления воды за день, для пользователя. В расчет берет физические упражнения за день, факторы (жарко, болезнь).
Класс считает норму и сколько уже выпито воды.
Стали-бы вы делить класс на 2?

pozitiffcat ★★★ ()
Ответ на: комментарий от pozitiffcat

ну так а почему бы сразу не разделять и не делегировать?) Мне гораздо удобнее, когда модель - это просто DTO, а логика в репозиториях и сервисах. Во-первых, единообразие, и в этих терминах думаю не только я, но и команда. Во-вторых, не надо думать о границе, когда выделять, а когда нет. У тебя тут этот класс ещё логи не пишет может и логи добавить? И я таки не понял, зачем мне создавать соединение какой-то сервис и логгер на каждый объект «книга», может я их 1кк хочу создать. И что если это надо развернуть в каком-нибудь тестовом окружении, где нет никакого принтера? Код менять или сам принтер об этом знать должен? Не проще ли сразу все правильно сделать, это ведь не сложно?

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

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

Если это хеллоуворлд или из серии написал за 20 минут и забыл, то непонятно, о чем речь и зачем вообще спрашивать. Если это проект, который будет поддерживаться, то в продукт хаос приносить не стоит на пустом месте.

nguseff ()
Ответ на: комментарий от pozitiffcat

Исходя из ТЗ — нет не надо. Но если пойти дальше, то вообще не очевидно зачем делать методы не static.

У Дяди Боба есть хорошая статья (и по моему даже доклад) на тему SRP.

A1 ()
Ответ на: комментарий от pozitiffcat

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

nguseff ()
Ответ на: комментарий от pozitiffcat

А вдруг логика будет меняться по настройке? Нам же нужно предусмотреть ВСЕ!

Вот когда появится настройка, тогда и отрефакторишь. Ты реально не видишь разницы между двумя примерами?

A1 ()
Ответ на: комментарий от A1

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

pozitiffcat ★★★ ()
Ответ на: комментарий от pozitiffcat

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

A1 ()
Ответ на: комментарий от pozitiffcat

Чем тогда ОО языки будут отличаться от процедурных

Еще один начинает прозревать))

A1 ()
Ответ на: комментарий от pozitiffcat

Какая разница ОО языкам сколько функций в классах? Как это с принципами ООП или SOLID связано?

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

nguseff ()
Ответ на: комментарий от A1

Это заход не стого конца. Юнит тесты не корректируют плохую архитектуру, а требуют большей гибкости. Самому приложению возможно это совсем не нужно.

Booster ★★ ()
Ответ на: комментарий от nguseff

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

pozitiffcat ★★★ ()
Ответ на: комментарий от nguseff

Это все правильно, но в реальности все это может быть не нужно. Как правило вся эта архитектура очень нужна всяким библиотекам, в приложении же это может быть архитектурой ради архитектуры. Красиво - да, но работает и так и так.

Booster ★★ ()
Последнее исправление: Booster (всего исправлений: 2)
Ответ на: комментарий от Booster

Это заход не стого конца.

Да, пожалуй, но легко тестируемый код, как правило, обладает хорошим дизайном. Сразу убиваем двух зайцев.

A1 ()
Ответ на: комментарий от Booster

В реальности потом начинается бред. Люди сидят и руками тестят функции целый день вручную меняя в коде какие-то параметры, снова перекомпилируя, запуская, заходя в интерфейс программы, инициируя ошибку и т.д. Появляются куски кода, которые все боятся трогать и никто не понимает, что они делают. Тестами их покрыть невозможно, а менять страшно, потому что всё упадет потом сразу в тысяче мест. Логика, которую делает один класс в 2к строк уже не умещается в голове программиста. Поддержка/тестирование превращается в сущий ад.

nguseff ()
Ответ на: комментарий от Booster

Красиво - да, но работает и так и так.

SOLID не про «красиво». Это достаточно практичные принципы. Опять же, если код покрыт тестами -> его можно покрыть тестами -> архитектура это позволяет -> это более хорошая архитектура, чем в примере автора.

nguseff ()
Ответ на: комментарий от pozitiffcat

тут да. Не всегда нужно искать максимально гибкое решение. Код фреймворков может быть сложен из-за гибкости, при этом делать простые вещи. Бизнес код не всегда должен быть гибким, он должен быть тестируемый и поддерживаемый.

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

nguseff ()
Ответ на: комментарий от nguseff

Бред или нет - понятие относительное, везде нужно знать меру. Не стоит делать мегаклассы, как и не стоит делать мильон. SOLID это вообще смех - несколько принципов выдранных ради того, чтобы получилось красивое слово.

Booster ★★ ()
Ответ на: комментарий от Booster

Только юнит тесты не делают автоматически архитектуру хорошей.

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

A1 ()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.