Комментарии 21
Хорошая статья
Возможно стоит подробнее обсуждать минусы решений в некоторых сносках, например вот это решение вызовет проблемы при синхронизации по сети, а синглтоны конфликтуют с тестами и могут вызвать проблемы с пользователем как с сущностью (для сейв лоада)
Но вообще вопросы хорошие - в сети описано много по хорошему коду, но мало по хорошей архитектуре, хорошим модулям и компонентам.
И тут раскрывается главное - не бывает хорошего в вакууме, есть много приемов, которые хороши для тех или иных требований и имеют свою цену.
Практичный подход помогает.
У меня статья вызывает больше недопонимания. Оговорюсь сразу, я очень далек от Unity.
Не существует никакого "идеального" инвентаря. Любая разработка начинается с определения набора требований. Как я должен понять требования к инвентарю по описанию эффекта Даннинга-Крюгера и некой непонятой картинке инвентаря? Что означает каждая секция инвентаря? Почему у ячейки инвентаря нет количества предметов?
Почему абстрактный класс BaseItem без ключевого слово abstract? Зачем вообще строить какую-то невнятную иерархию наследования без обозначения, кто его наследники? Какая вообще проблема решается через наследование? Почему просто не создать структуру только для чтения под названием InventoryCell (BaseItem - слишком абстрактное название)?
Почему ID у BaseItem задан как long? Я вижу сходу две явные реализации ID: как Enum (когда заранее известны все возможные предметы) и как еще одна структура только для чтения (когда все виды предметов загружаются динамически из ресурсов, когда надо валидировать ID и чтобы не дать возможность ID присвоить, скажем, CellID).
Почему количество ячеек в инвентаря определяется динамически, если на картинке нет полос прокруток?
Почему у Inventory ровно один интерфейс с очень странным названием IInventoryActions? Почему не просто IInventory? Зачем этот интерфейс вообще нужен, если в классе User он не используется? Почему IInventory не наследуется хотя бы от IEnumerable? Почему не используется индексатор для доступа к ячейке инвентаря? Зачем Add и Remove возвращают bool? Я должен поверить на слово? Это прямое нарушение YAGNI и принципа единственной ответственности из SOLID.
UIInventoryWindow. С какой целью абсолютно все поля во всех наследниках MonoBehaviour помечены атрибутом SerializeField, чтобы было, вдруг пригодится? Почему вообще вызывается Instantiate для ячейки, неужели нельзя обойтись без этого? Почему инвентарь не попадает в UIInventoryWindow через инверсию зависимости?
Почему UIInventoryCell содержит OnClickCell? Где вообще было сказано, что по ячейкам кликают мышкой? Почему OnClickCell не часть BaseItem? Почему у события неправильное название? OnCellClicked - это название для обработчика события, а не для события. Зачем вообще нужно это событие, какая проблема решается? У такой реализации событий очень много проблем, что проще забыть вообще про ключевое слово event. Одна из проблем прямо наглядно видна в методе OnPointerClick с ненужной проверкой на null.
Почему у класса User нет интерфейса? Почему приватное поле названо с большой буквы? Почему User знает как создавать инвентарь и самого себя? Почему не используется современный C# (или вовсе избавиться от приватного поля):
public static User Current
{
get => _current ?? new User();
}И что с того, что в играх на одного человека всего один игрок? Как этот код поддерживать? Как написать тесты на этот код? Про существование шаблона одиночка лучше вообще забыть.
ID чего принимает GetData в интерфейсе IStorageActions? Зачем здесь вообще обобщение используется?
В классе ItemVisualData (плохое название, класс описывает информацию о предмете) можно повесить атрибут SerializeField на свойство. Зачем вообще нужен этот атрибут? Для сериализации он не нужен. Почему VisualName - это строка, а не ID ресурса локализации? Статическое и только для чтения - это разные слова с разным смыслом, т.е. "статическое хранилище" - вообще не означает "только для чтения".
Зачем в классе ItemsVisualDataStorage метод Load? Не проще ли перенести содержимое метода в конструктор? И не надо в словаре вызывать метод ContainsKey, достаточно использовать индексатор.
ScriptableObject - это очень ужасное дизайнерское решение разработчиков Unity, что-то сопоставимое с шаблоном одиночка. Не надо в коде использовать сокращения, такие как SO. Почему эти данные не берутся из ресурсов?
Для тестов надо писать тесты, а не портить класс User.
Storage и Inventory. Раз приходится пояснять, почему это не одно и тоже, значит в коде что-то не так. У меня прослеживается логика, что Inventory хранит ячейки инвентаря, а Storage хранит описания предметов. Т.е. Storage - изначально неправильное название сущности + класс занимается загрузкой данных, чем нарушает принцип единственной ответственности из SOLID и отсутствие инверсии зависимостей очень сильно усложняет тестирование такого кода. Почему это не репозиторий, который возвращает обычный массив?
Что такое null в классе UIInventoryCell? Есть же хотя бы шаблон Null Object, который упростит код в методе SetItem. То, как достается иконка в методе, тихий ужас.
DragContext. Как быть, если инвентарь пользователя взаимодействует с инвентарем ящика?
Не надо вызывать Instantiate в классе UIInventoryDragContainer. Зачем использовать Tuple, когда в Unity поддерживается C# с нормальными именованными кортежами, а не вот это вот Item1.
Зачем вводить IDynamicStorageActions, когда уже был обобщенный класс для хранения?
Не надо было в класс User добавлять в свойство вызов метода. Следовало в таком случае отказаться от свойства вообще. И не надо смешивать доменную модель с способом хранения: атрибут JsonProperty не к месту и нарушает принцип единственной ответственности в SOLID. Не надо делать такую жесть как сохранения пользователя на изменение инвентаря. Или возвращение false, когда ничего не удалось загрузить. Никто не вызовет метод Load, если загружать нечего.
Хороший ли это код? Ну не знаю, на мой взгляд код ужасен. Как-то ожидаешь лучшего от Master of Unity3D, а не статью на непонятном языке: "монобехи", "SO", ридонли, иммутабельно, стейт и так далее. И не решена изначальная проблема на самой первой картинке с особым размещением ячеек по нескольким сеткам.
Эта проблема и не решалась. Хотя в рамках текущей архитектуры решается базово имплементацией второго инвентаря. Но вот про это в статье я и говорю :)
Писать про архитектуру дело неблагодарное. Слово abstact допустим. А что это меняет с точки зрения архитектуры?) Синтаксическое ограничение того, что класс абстрактный в контексте примера?)
Половина списка - это вкусовщина, которая никак не влияет не на расширение, ни на эксплуатацию реального решения. Про SRP вообще в целом не правда, так как атрибут не добавляет дополнительной ответственности классу. Особенно такой, которая создавала бы проблемы. Вот поэтому я и не пишу про хороший код, так как это просто неблагодарный труд. Так как придут люди без продуктового обоснования почему, а просто со вкусовыми предпочтениями :)
Но давайте я по отвечаю
Не существует никакого "идеального" инвентаря. Любая разработка начинается с определения набора требований. Как я должен понять требования к инвентарю по описанию эффекта Даннинга-Крюгера и некой непонятой картинке инвентаря? Что означает каждая секция инвентаря? Почему у ячейки инвентаря нет количества предметов?
Потому что я не решаю задачу с картинки и не заявлял этого темой статьи
Почему абстрактный класс BaseItem без ключевого слово abstract? Зачем вообще строить какую-то невнятную иерархию наследования без обозначения, кто его наследники? Какая вообще проблема решается через наследование? Почему просто не создать структуру только для чтения под названием InventoryCell (BaseItem - слишком абстрактное название)?
Стандартный подход в геймдеве. Да это может быть класс контейнер в дальнейшем компонентном подходе. Это классика, это при разработке 5-6 взрослых игр знать надо
Почему количество ячеек в инвентаря определяется динамически, если на картинке нет полос прокруток?
Потому что это абстрактный инвентарь. Когда мы пишем по MVVM в целом странно такие вещи определять на каком-то уровне кроме View. Зачем ограничивать размер инвентаря в изначальной архитектуре системы? Для какой цели?
Почему у Inventory ровно один интерфейс с очень странным названием IInventoryActions? Почему не просто IInventory? Зачем этот интерфейс вообще нужен, если в классе User он не используется? Почему IInventory не наследуется хотя бы от IEnumerable? Почему не используется индексатор для доступа к ячейке инвентаря? Зачем Add и Remove возвращают bool? Я должен поверить на слово? Это прямое нарушение YAGNI и принципа единственной ответственности из SOLID.
Название - мой кодстайл. Это не нарушает единственность ответственности. Это нарушает что каждая фигня должна быть реализована, с чем я изначально в солиде не совсем согласен. А изначально это заложено для асинхронных операций, чтобы при доступе на сервер или куда-то ещё при ошибке авторизации или сетевых ошибках знать, что что-то пошло не так. Но я бы написал TryAdd, а не просто Add, это было бы вернее
Почему UIInventoryCell содержит OnClickCell? Где вообще было сказано, что по ячейкам кликают мышкой? Почему OnClickCell не часть BaseItem? Почему у события неправильное название? OnCellClicked - это название для обработчика события, а не для события. Зачем вообще нужно это событие, какая проблема решается? У такой реализации событий очень много проблем, что проще забыть вообще про ключевое слово event. Одна из проблем прямо наглядно видна в методе OnPointerClick с ненужной проверкой на null.
Это потом выпиливается. Но если брать до того, чтобы ячейка не знала про окно, но окно рулило кросс взаймодействием как контекст. Потом я решил контекст завести отдельно
Почему у класса User нет интерфейса? Почему приватное поле названо с большой буквы? Почему User знает как создавать инвентарь и самого себя? Почему не используется современный C# (или вовсе избавиться от приватного поля):
Опечатка, не относится к архитектуре. Сахар, тоже не относится к архитектуре и мелочь. Рихтер в целом считает сахар злом. Вкусовщина
И что с того, что в играх на одного человека всего один игрок? Как этот код поддерживать? Как написать тесты на этот код? Про существование шаблона одиночка лучше вообще забыть.
Прикольное заблуждение. Проекты зарабатывающие миллионы долларов, которые не тонут в техдолге при наличии одиночки как-то существуют. Я в целом не согласен с критикой. Но тут ошибка концептуальная, так как у меня не одиночка и оно спокойно мокается. Там просто статик контейнер
ID чего принимает GetData в интерфейсе IStorageActions? Зачем здесь вообще обобщение используется?
Доступ к статическим ресурсам, чтобы получить единицу даты по неопределённому алгоритму. Сторажд может быть SO, на диске, сетевой реализацией и много чем ещё
В классе ItemVisualData (плохое название, класс описывает информацию о предмете) можно повесить атрибут SerializeField на свойство. Зачем вообще нужен этот атрибут? Для сериализации он не нужен. Почему VisualName - это строка, а не ID ресурса локализации? Статическое и только для чтения - это разные слова с разным смыслом, т.е. "статическое хранилище" - вообще не означает "только для чтения".
Коммент не знания контекста юнити. Он нужен для Unity сериализации. Статическое хранилище в моём контексте ридонли ресурсы, не изменяемое в рантайме
ScriptableObject - это очень ужасное дизайнерское решение разработчиков Unity, что-то сопоставимое с шаблоном одиночка. Не надо в коде использовать сокращения, такие как SO. Почему эти данные не берутся из ресурсов?
Это оболочка над текстовым файлом с готовым интерфейсом. Обычный статический ресурс и в коде он берётся из ресурсов, а не синглтоном. Просто не умение пользоваться Unity. Это как ругаться на ньютонсофт json для хранения статических конфигов.
Для тестов надо писать тесты, а не портить класс User.
Не понимаю вообще к чему и почему
Storage и Inventory. Раз приходится пояснять, почему это не одно и тоже, значит в коде что-то не так. У меня прослеживается логика, что Inventory хранит ячейки инвентаря, а Storage хранит описания предметов. Т.е. Storage - изначально неправильное название сущности + класс занимается загрузкой данных, чем нарушает принцип единственной ответственности из SOLID и отсутствие инверсии зависимостей очень сильно усложняет тестирование такого кода. Почему это не репозиторий, который возвращает обычный массив?
Не нарушает солид, банальное разделение сущностей по доменам если уж мы в термины пойдём подобные. Это два несвязанных домена, поэтому не стоит их обобщать
Что такое null в классе UIInventoryCell? Есть же хотя бы шаблон Null Object, который упростит код в методе SetItem. То, как достается иконка в методе, тихий ужас.
Хотелось бы пояснение в чём. Null - отсутствие предмета. Ничё особеннного
DragContext. Как быть, если инвентарь пользователя взаимодействует с инвентарем ящика?
Использовать тот же контекст
Не надо вызывать Instantiate в классе UIInventoryDragContainer. Зачем использовать Tuple, когда в Unity поддерживается C# с нормальными именованными кортежами, а не вот это вот Item1.
Почему не надо вызывать? Тоже вкусовщина не относящаяся к архитектуре.
Зачем вводить IDynamicStorageActions, когда уже был обобщенный класс для хранения?
Мутабельные и иммутабельные данные. Просто так удобнее разделить
Не надо было в класс User добавлять в свойство вызов метода. Следовало в таком случае отказаться от свойства вообще. И не надо смешивать доменную модель с способом хранения: атрибут JsonProperty не к месту и нарушает принцип единственной ответственности в SOLID. Не надо делать такую жесть как сохранения пользователя на изменение инвентаря. Или возвращение false, когда ничего не удалось загрузить. Никто не вызовет метод Load, если загружать нечего.
На это я ответил выше. Оно ничего не нарушает (хотя это дискуссионный вопрос), просто удобство сериализации данных юзера, чтобы не выносить их в отдельные классы с данными. Это уже спорный момент, но не ведущий ни к каким проблемам с точки зрения проектировки
Проблема в том, что нигде в статье не были описаны требования к инвентарю, кроме картинки, чтобы говорить о чем-то конкретном. В статье есть очень удачное предложение, которое характеризует большинство моих пунктов, что до ввода MonoBehaviour можно было играть в консоли. Они описывают тесную связь логики и представления с примесью статических классов. Никакие интерфейсы, абстракции и события в отделении логики от представления не помогут в этом случае, если их использовать таким образом. Код же необходимо писать таким образом, чтобы он был понятен другому разработчику, а не потому что и так все работает. Большую часть времени код читается, а не пишется.
В User нарушен SRP, потому что он управляет жизненным циклом инвентаря, управляет сохранением и загрузкой состояния, прибит гвоздями к конкретной реализации сериализации. Атрибут Serializable не требует наличие атрибута JsonProperty для сериализации классом JsonUtility, а сериализация классом JsonConvert не требует атрибут JsonProperty для сериализации всех свойств. Т.е. добавляется еще головная боль с тем, а всё ли можно засериализовать, и с тем, что нужно сериализовать, а что нет.
Касательно Null Object, речь про то, что у объекта два состояния: объект есть и объекта нет. Поведение с иконкой вылезло за границы объекта. Я знаю, что в Unity такое обычно решается через шаблон состояние.
Instantiate и зарабатывание миллионов долларов - связанные вещи. Мне так надоело, когда в том же Steam, появляется очередная игра усеянная багами, что иногда даже запустить ее невозможно. Конкретно здесь речь, когда условный тетрис (очень простая игра), отжирает огромное количество ресурсов или падает при странных условиях. Некоторые разработчики даже не удосуживаются банально проверять локаль системы, из-за чего падает DateTime, зато продают игру за 30 долларов. ААА игры тоже страдают от слабого уровня программистов, например Wasteland 3 долгое время грузил уровни по несколько минут, сбрасывал прогресс при загрузке сохранения. Главное верить, что озлобленные потребители не вернут деньги и не перестанут покупать будущие игры.
ScriptableObject обычно используется именно так, как я описал. Бывает даже в него домешивают поведение, никто же не запрещает. Был даже один довольно популярный доклад, который предлагал все на них делать. Такая веселая сущность, что даже сбрасывает все свои свойства при небольшом рефакторинге.
Я не обозначал целью статьи конкретную реализацию инвентаря. Все перекладывания и т.п. в текущей реализации вообще делаются на уровне префабов при желании. Часть с мелкими доработки. Изначальный тезис статьи, не бывает идеального кода в абстрактном случае. А в конкретном случае писать нет смысла, так как у каждого такой конкретный случай свой. И исходя из своего опыта к любому коду можно придраться, даже если он целиком решает задачу.
Да не нарушен, он управляет жизненным циклом юзера. А интерфейс это часть данных юзера. Это вполне нормальный контейнер обобщение.
Да не нужен тут шаблон состояние, это пере усложнение бизнес логики ни за чем.
Про Instantiate. Ну не зная как работает в Unity GC и в целом сборка мусора, а так же Destroy, и что он в этом случае будет делать, то конечно можно представить невообразимые ужасы. В в данном контексте использования объект будет небольшим и удалится всего лишь ямль конфиг, который соберётся и при стратегии работы с большой кучей не вызовет никогда проблемы. Можно оптимизировать, когда понятна конкретика и контекст.
Про SO, ну кем-то используется, ток я его кажется использую чисто как статический ресурс. В шарпе тоже много вещей, которые нужно когда юзаешь понимать, как их стоит юзать)
В моём понимании, реализацию чего-то идеального возможна, но не всегда оправдана по деньгам. Даже в системе с инвентарем, мы можем написать два класса для разных жанров (онлайн), напихать дженериков и написать некий абстрактный интерфейс для доступа к каким-то полям и жить себе спокойно. Просто это займёт до 4 раз больше времени на проектирование, что немного тупо и в целом ничего не поменяет.
Вам сделали шикарный код ревью, причем бесплатно. Вы бы хоть спасибо сказали.
Эта проблема и не решалась
Теперь понятно, откуда у нас столько тормозящего софта. Вместо того, чтобы решить конкретную проблему, будет делать абстракции над абстракциями.
https://www.joelonsoftware.com/2001/04/21/dont-let-architecture-astronauts-scare-you/
Проблемы на самом деле начинаются уже с фразы
Конечно же нет. Переписываем класс, так как имени и иконки тут не должно быть.
Это класс данных. Имя (на самом деле это конечно ID ресурса локализации, он это уже низкоуровневые подробности, которые могут и должны быть сокрыты в типе поля Name) и иконка как раз тут должны быть. А вот рисовать ли их в каждом конкретном случае и как это делать уже решает слой визуализации.
Почему абстрактный класс BaseItem без ключевого слово abstract? Зачем вообще строить какую-то невнятную иерархию наследования без обозначения, кто его наследники? Какая вообще проблема решается через наследование? Почему просто не создать структуру только для чтения под названием InventoryCell (BaseItem - слишком абстрактное название)?
Стандартный подход в геймдеве. Да это может быть класс контейнер в дальнейшем компонентном подходе. Это классика, это при разработке 5-6 взрослых игр знать надо
Я поддержу второго комментатора. Чтобы применять наследование, нужна весомая причина. Данные в инвентаре — это модели, у них я бы не стал делать разное поведение, или какое-то поведение в принципе.
Но ваша аргументация в духе "все так делают, это знать надо" оставляет меня желать лучшего.
Но я радуюсь, когда в моделях предлагают не хранить графику и локализацию. Разделение — это хорошо.
Я щас подумал, что в старых играх я видел мешочки: такой элемент в инвентаре, куда предметы можно засовывать. То есть это предмет с предметами.
В таком случае можно использовать наследование для разделения простой ячейки с предметом и ячейки-контейнера с кучкой предметов. Но можно и не через наследование это сделать.
И в таком случае этот класс IItemCell будет внутренней реализацией внутри структуры инвентаря. А класс, непосредственно описывающий предмет инвентаря, наследовать какой-либо абстрактный класс не должен.
Дак дело не в аргументации. Дело в том, что я говорю по компонентный подход. У меня наследование даже не используется. По сути про наследование только говорится :)
Аргументация может быть эмоциональной. А теперь давайте пойдём в ООП. Предмет это абстрактный класс или же интерфейс? Так как это объект с данными, а не объект поведения - это абстрактный класс. А вот стоит ли навешивать функциональность компонентами или же делать это через наследование - вопрос к системе. Разное поведение, Ну предположим у нас в инвентаре есть шмот перса, а есть расходники. Это объект одного класса? Если да, то почему и зачем? На клике что происходит, кто определяет и как. расходник не является предметом или шмотка не является предметом.
Просто вот реальный косяк тут тоже есть, но его будто бы никто не замечает, что меня огорчает. И это IEnumerable в интерфейсе инвентаря, но при этом я завязываюсь на логику индексов в идентификаторах ячеек. То есть по сути определяю выше поведение списка, и заменив реализацию на уровне инвентаря - всё сломается и будет баг. И это ошибка проектировки. Причём довольно стандартная)
А многие делают поведения компонентами. Вопрос не в том, что все знают. В этом я выразился некорректно, согласен. Вопрос в том, что обсуждаемое, мантры из SOLID, и т.п. не говорят об одной важной вещи. В чём конкретно проблема этого подхода? Почему стоит сделать иначе? Какой выигрыш это даст?
Суть не в паттернах, солиде, MVC, MVVM и т.п. А в том "для чего". Пример ужасного кода я так же могу показать, но суть в том, что код это далеко не он.
Предмет это абстрактный класс или же интерфейс? Так как это объект с данными, а не объект поведения - это абстрактный класс.
Не могу согласиться. Предмет в инвентаре — это модель, ни больше, ни меньше. Совсем не обязательно использовать абстрактные классы и наследование для разделения объектов по общим значениям в модели. Я, например, в моделях никогда вообще не использую наследование, методы, абстракции.
Вот ниже пример, как предметы героя описываются простым классом. Теги — лишь как один из способов хранить дополнительные особенности каждого предмета.
itemType поле описывает основную разницу между предметами. По типу легко определить, что с этим предметом можно делать. Но можно вообще на одних только тегах все описать. Set — это лист, массив, в котором не могут повторяться одинаковые значения, а порядок не важен.
@immutable
class ItemModel {
final String id;
final ItemType itemType;
final Set<Tag> tags;
}Ну предположим у нас в инвентаре есть шмот перса, а есть расходники. Это объект одного класса? Если да, то почему и зачем?
Да, у них одинаковое поведение. Они просто лежат в кармане героя, пока их не достанут. У них разный тип, но тип совсем не обязательно указывать через класс. Можно взять enum.
На клике что происходит, кто определяет и как. расходник не является предметом или шмотка не является предметом.
"Пользователей" предмета полно. Это может быть окно крафта, окно продажи, окно экипировки героя. И везде будут разные функции на "клик" или "drag-n-drop". Вот тот, кто вызывает из памяти список предметов, тот и говорит, что он хочет делать с предметами.
Класс модели совсем не должен содержать поведения, только описание предмета. Поведение делает бизнес-логика.
Скорее всего это просто функция, которая по описанию предмета и определяет, как с ним сделать что-то: какие возможные действия предложить, какую цену на продажу выставить, на какую часть тела это можно одеть. На каждую задачу своя функция, своя логика.
Например, вот способ подсчета стоимости предмета:
double getItemPrice(ItemType type, Set tags) {
late double price;
switch (type) {
case ItemType.sword01:
price = 12.0;
}
if (tags.contans(Tag.broken)) price = price * 0.1;
if (tags.contains(Tag.rare)) price = price * 10;
}Вопрос в том, что обсуждаемое, мантры из SOLID, и т.п. не говорят об одной важной вещи. В чём конкретно проблема этого подхода? Почему стоит сделать иначе? Какой выигрыш это даст?
Я понимаю, что можно и через ООП и наследование все описать, или еще какими-нибудь другими способами, двумерными массивами, например, вообще без каких-либо классов. Тут нет явно правильного и неправильного варианта. Просто у каждого подхода будут свои особенности.
На своем опыте я пришел к тому, что наследование приносит много проблем, делает код слишком жестким к изменениям, его сложно поддерживать, приходится порою придумывать костыли для обхода механизмов наследования.
А что наследование дает взамен здесь? Я не знаю. Но чем меньше модель знает о вариантах поведения, тем лучше. Чем меньше связанности в коде — тем лучше. Чем гибче код — тем проще его изменять, это хорошо.
Да согласен. Тут нет вопросов, всё понятно и аргументировано. Спасибо. Я немного обсудил в своём канале в комментах к этому посту. Мой подход немного в другом, скопирую сюда часть, но суть та же. Enum это тоже не очень хорошо.
Вопрос:
Допустим потребуется у каждого предмета в инвентаре указывать его количество. Для реализации этого нужно будет добавить int Count в BaseItem? Или нужно будет отнаследоваться? Мне, честно, не очевидно, какой именно из вариантов подразумевается.
Или если посложнее - нужно будет добавить новую фичу - айтем-рюкзак. Как это реализовывать нужно будет? Наследоваться от BaseItem? Вводить какой-то enum в BaseItem, указывающий на тип предмета? Добавлять список компонентов и в этот список добавлять объект, отнаследованный от IInventoryActions? По-хорошему - над этим должен думать не исполнитель, которому поручена эта задача, а архитектор, изначально написавший этот код.
Т. е. моя претензия в том, что поручить кому-то независимо расширять этот код невозможно - исполнителю по-любому придётся связываться с архитектором, и в этом его косяк.
Ответ:
Да нет, Count в BaseItem добавлять нельзя. Это приведёт к тому, что придётся делать его Nullable, а обработка на null (или -1) из-за того, что предмет не имеет понятия Count везде — это плохо. Нужно отнаследоваться из этих двух выборов. А вообще нужно по хорошему завести свойство, которое запрашивается по айди из стораджа. И от него уже скачет реализация.
Миллион свойств, миллион обработок в ячейке на отображение и на клик?) Да нет, так как там будут только свойства обрабатываться относящиеся к инвентарю — это зависит уже от сложности реализации инвентаря. Чистое наследование — это скорее плохо, так как прийдётся либо тайпкастить, либо разделять ячейки на типы по типу предмета для отрисовки предмета. Логичнее, как и с иконкой идти по чекналл и отрисовке свойств в самой ячейке, вряд ли их там будет миллиард :)
И любой айтем можно таким образом добавить, так как вопрос лишь в том, как получить к нему данные, а для этого нам нужен только айди. А инвентарь занимается отрисовкой свойств относящихся к нему. И ему плевать что сумка-айтем умеет что-то ещё. Так что BaseItem надо было запечатать. А само по себе это для читаемости.
Это даже не класс модели. По сути этот класс можно было запечатать и свести к идентификатору. Называется он так для читаемости. Так как это по сути скелет, на который набрасываются свойства. Он даже не знает о существовании этих свойств, они набрасываются в зависимости от контекста использования. Как иконка. И дальше идёт базовый механизм расширения функциональности любого вью, без кучи "наследников вью, тайпкастов" и прочего мракобесия. Проверка обладает ли этот ID этим свойством, и если да то его отрисовкой в соответствующей панели. Так можно расширять до бесконечности от конкретики интерфейса. Просто есть стораджи разных свойств, эти свойства приписаны к идентификатором предмета. Вью или контроллеры запрашивают из стораждей (по сути моделей). "А у этого айди есть это свойство?" Если ответ да, то подключают нужную функциональность.
Просто это не делает код ужасным. Спорным в контексте конкретных применений - да. И как раз про это статья. Просто если скажем предметов у нас два типа или у них нет множества свойств. То это вообще не имеет значения. И можно наследоваться. А подход описанный мной сейчас - это оверинжиниринг.
Да, у предметов может быть много самых разных свойств, которые может быть трудно уложить в одну древовидную структуру.
Например, количество предмета, патронов. Можно описать поле countable, которое является union типом. Может выглядеть так:
class ItemModel {
final Countable countable;
final ItemType itemType;
ItemModel({this.type, this.countable});
//...
}
final ammo50 = new ItemModel(
type: ItemType.ammo50,
countable: Countable.many(137),
);
final int count = ammo50.countable.when(
single: () => return 1,
many: (count) => return count
);Этот union type штука удобная, из функционального программирования пришла. Отличная вещь!
Таким образом предметам можно описать много разных видов свойств, которые по характеристикам отличаются друг от друга. Можно разные варианты множественности вещей описать.
Например, патроны не просто в количестве 137 штук, а они в инвентаре должны отображаться пачками по 50. Тогда Countable может предусматривать третий вариант stacked с дополнительным полем stackMax = 50.
Удобно, потому что свойством "количество" могут обладать предметы из разных категорий. Не нужно будет делать сложные наследования, миксины. А самое главное, не нужно будет делать проверки на тип предмета.
Еще с наследованием проблема в том, что когда мы делаем проверки на тип, мы должны знать, какой список классов и интерфейсов проверяем. А если его надо обновить из-за того, что мы изменили структуру дерева наследования? А если таких мест в коде много, и их еще поискать надо все, чтобы не забыть?
Поэтому очень тяжело писать про хороший код, так как у каждого своё понятие хорошего кода, исходя из опыта.
Спасибо, Кэп!
И это в предпоследнем абзаце. Хотел посмотреть авторский ответ на вопрос из заголовка. Нашел авторское определение эффекта Даннинга-Крюгера, авторские размышления о какой-то игрушке с инвентарем, авторскую простыню кода. И немножко рассуждений, хорош ли конкретно код из простыни. Ну, спасибо и на том.
Меня периодически спрашивают, почему я так мало пишу про архитектуру
Навеяло
"Ко мне обратилась группа товарищей из молодежи с предложением высказать свое мнение в печати по вопросам языкознания..."
А потом у нас появятся предметы, которые занимают более одной ячейки...
"Почему тяжело писать про хороший код?"
- Ревью кода в комментариях показывает почему.
У меня самого давно сложилось мнение и ощущение, того, что в большинстве своём в гейм-деве происходит тихий ужас под капотом. Реализации намертво прибиты к чему-то, очень плохая выстроенная система, SOLID, паттерны и т.д. и т.п. редко применимы. Подавляющая часть видео, уроков, статей, смотришь код и страшно. Начиная с банального именования. Такое ощущение что гейм-дев постоянно отстаёт от современной разработки.
Имхо писать надо стараться нормальный читаемый код, хорошо сразу не получится, нет пределов совершенству. Доводить его до хорошего да стоит, иначе разгребать тех долг через какое-то время станет больно
Почему тяжело писать про хороший код?