Соответсвует ли код принципам солид?

Question

[Герман Коффман]

Есть код, который написан был мной по принципам solid и использованием ооп. Вопрос в том, все ли правильно и чтобы вы посоветовали исправить или добавить? Заранее прошу прощение за всю кровь которая у вас вытечет из глаз, присылайте группу, отправлю почтой)
Клик

Comments

[Василий Банников]

K0TlK, перенеси в ответы и пингани меня. Отмечу решением.

[K0TlK]

Василий Банников, пингую

Answer 1

[K0TlK]

Разделять код на миллион монобехов != ооп. Единственный способ достичь правильного ооп в юнити - отделять всю логику от движка, а монобехи использовать как вьюшку, чтобы отображать всякое. Но реализуемый для тебя способ - использовать интерфейсы, под каждый паблик метод отдельный интерфейс, взаимодействовать через эти интерфейсы, учиться нормально прокидывать зависимости: всегда должна быть какая-то точка входа, где будут инициализироваться компоненты и передаваться все зависимости.
Virtual методы = плохо, класс в идеале должен быть либо abstract либо sealed.
Protected поля = public поля = нарушение инкапсуляции/иммутабельности.
PlayerHealth, PlayerEnergy, Health - это все одно и то же с разными реализациями, есть интерфейс IHealth все под него и просто этот интерфейс реализуешь.
AudioPlayer вообще какой-то ужас. Во-первых, почему GameObject takeDamageSourseObject и т.д Почему GameObject, если ты потом получаешь у него компонент AudioSource, ты можешь конкретные компоненты в инспекторе передавать, т.е. не GameObject, a AudioSource и GetComponent потом делать не нужно будет. Во-вторых, из всего этого можно было сделать один компонент, в котором будет один метод Play, который будет принимать AudioSource и проигрывать его. Либо напрямую пропихивать AudioSource и воспроизводить звук.
Нет какого-то единого кодстайла, где-то есть нижнее подчеркивание, где-то нет, где-то есть _cs где-то нет, для чего эта _cs я так и не понял. Где-то сериализуемое приватное поле, где-то тупо паблик.
Про солид вообще смысла говорить нет. Интерфейсов пара штук на весь проект. Зависимости нормально не прокидываются, всё через поля.
Это так, навскидку. Думаю, если зайти в папку bot, то можно будет диссертацию написать по содержимому этой папки.

Comments

[Герман Коффман]

Разделять код на миллион монобехов != ооп.

Это да, но это было сделано с целью соблюсти принцип единой ответсвенности.

AudioPlayer вообще какой-то ужас. Во-первых, почему GameObject takeDamageSourseObject и т.д Почему GameObject, если ты потом получаешь у него компонент AudioSource, ты можешь конкретные компоненты в инспекторе передавать, т.е. не GameObject, a AudioSource и GetComponent потом делать не нужно будет.

Понял. Мне казалось так будет просто удобнее. Мол кинул еще один AudioSourse, закинул звук до старта и он уже тоже рандомно выберается.

Во-вторых, из всего этого можно было сделать один компонент, в котором будет один метод Play, который будет принимать AudioSource и проигрывать его.

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

Нет какого-то единого кодстайла, где-то есть нижнее подчеркивание, где-то нет, где-то есть _cs где-то нет, для чего эта _cs я так и не понял. Где-то сериализуемое приватное поле, где-то тупо паблик.

И то и то связано с тем что я эксперементирую и сам еще не решил что нужно использовать. Ну и конечно постоянные правки и переименования в которых тоже что то теряется.

Про солид вообще смысла говорить нет. Интерфейсов пара штук на весь проект.

Если честно - даже не знаю куда их столько. Нету идей куда новые пихать.

Зависимости нормально не прокидываются, всё через поля.

Про это если честно не понял.
Про приватные поля - да. Вопрос в том противоречит ли ооп создания публичных переменных если у них get и set публичные? Или все должно быть как в примере с помощью функции Get() и функции Set()?

PS: спасибо, очень приятно видеть твои ответы. Они всегда годные и по делу!

[Василий Банников]

Герман Коффман,

Это да, но это было сделано с целью соблюсти принцип единой ответсвенности

Разбиение на мелкие кусочки - это не SRP и даже не interface segregation.

Про это если честно не понял.
Про приватные поля - да. Вопрос в том противоречит ли ооп создания публичных переменных если у них get и set публичные? Или все должно быть как в примере с помощью функции Get() и функции Set()?

Публичная переменная в любом виде (поле, свойство, Get/Set) уже повод задуматься. Если можно вместо переменной дать осмысленное название операции - лучше дать осмысленное название.

[K0TlK]

Герман Коффман,

Или все должно быть как в примере с помощью функции Get() и функции Set()?

Публичные поля/свойства/методы get, set это все нарушение иммутабельности/инкапсуляции объекта. Это то же самое, если бы у тебя сердце можно было просто достать рукой и заменить его на другое. Удобно, но это может сделать кто угодно, когда угодно, откуда угодно.
Объект это не мешок с данными и методами для взаимодействия с ними. Взаимодействовать нужно с объектом, а не с данными, которые он содержит.
Как пример:

public class Document
{
    public string Text { get; set; }
}

В этом случае Document просто какое-то DTO(Data transfer object), а не объект, взаимодействие с ним будет соответствующим:

public class Human
{
    public void ReadDocument(Document document)
    {
        Debug.Log(document.Text);
    }
}

Human взаимодействует с данными, которые содержит Document, а не с самим объектом Document.
И другой пример:

public class Document
{
    private readonly string _text;

    public Document(string text)
    {
        _text = text;
    }

    public void Print()
    {
        Debug.Log(_text);
    }
}

Теперь объект Document иммутабельный. Его текст устанавливается единожды при создании объекта, через конструктор. Изменить это поле нельзя, поэтому он всегда будет сохранять свое состояние. И, благодаря методу Print теперь не нужно взаимодействовать с данными объекта напрямую. Соответственно, теперь взаимодействие будет происходить с объектом, а не с его данными:

public class Human
{
    public void ReadDocument(Document document)
    {
        document.Print();
    }
}

[K0TlK]

Герман Коффман,

И то и то связано с тем что я эксперементирую и сам еще не решил что нужно использовать. Ну и конечно постоянные правки и переименования в которых тоже что то теряется.

Тебе и не надо экспериментировать, есть конвенции, следуй им:
https://docs.microsoft.com/en-us/dotnet/csharp/fun...
https://github.com/dotnet/runtime/blob/main/docs/c...

Answer 2

[mayton2019]

Есть такой анекдот что на 100 строк разработки Java приходится 10 строк Clojure с точно таким-же
алгоримическим смыслом. Так вот мне кажется что в данном исходнике этот коэффицент еще худе.

Код - по большей части ничего не делает. Он настолько формален и общ, что мне кажется что 50% callbacks можно заинлайнить и кода станет меньше а читаемость пострадает не сильно. Вобщем - редкий случай когда SOLID вместо помощи разработчику - создаёт ненужные абстракции.

У кода - очень неравномерная плотность информации. Например в Weapons/Bow.cs есть метод BallisticVel который резко контрастирует с другим кодом. Тут - как будто клавиатуру взял другой человек и написал в Haskell-style формулу. У меня возникает вопрос. Почему автор так старался декомпозировать всякий формализм а сложную функцию не декомпозировал? Вобщем такая резкая смена плотности информации на квадратный метр исходников - очень настораживает.

В качестве метрики "полезности" - я-бы спросил автора

- Ты бы сам себе заплатил-бы за такой код?

Comments

[Герман Коффман]

Про callbacks - понял. Делал это с целью исполнения правила приватности солид, уберу их где можно. Про Bow услышал, надо будет сделать какой нибудь класс калькулятора, где все это можно будет считать и функцию которая не требует класса на обьектах сцены, так и правда будет лучше. Интересный вопрос)
Есть еще какие-нибудь мыли по поводу улучшения качетва кода? Получилось ли хотя бы приблизиться к исполнению принципов солид?

[mayton2019]

Герман Коффман, SOLID это не требование, а философия. Поэтому его соблюдать не обязательно.

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

И покрыть тестами хотя-бы расчетные формулы. Показать свойтва. Например стрела из лука летает с конечной скоростью. И неотрицательной скоростью. Вообще придумай сам.