Всем привет! Меня зовут Дядиченко Григорий, и я технический продюсер. В своём блоге в телеграм я периодически публикую задачки по Unity. Решение одной задачки получается слишком длинным, чтобы писать про него в блог. Поэтому я решил это оформить в статью. Задачка звучит так.
Вы решили сделать с друзьями свой скайрим. У вас пока в игре есть два вида оружия: молот и меч. Они отличаются уроном, прочностью и скоростью атаки. Плюс логикой атаки. Атака молота аое в круговой зоне. Атака меча бьёт до 3 ближайших противников. К вам пришёл катжит и за лунный сахар предложил купить у него схему архитектуры на картинке выше. Предложите решение лучше. В чём проблема данной схемы?
Я понял ошибки своей прошлой статьи про архитектуру и тут уже обозначил условия задачи конкретнее. В архитектурной схеме выше достаточно много проблем, но пойдём мы по порядку. Моё решение не является единственным, но тут всё же нужно учитывать ряд ограничений по условиям.
Определение сущности объекта через наследование
Есть старая задачка по C++ про зоопарк и тигров. И она неплохо объясняет концепцию наследования, но в ней та же проблема. Если доводить до абсурда, то в мире насчитали более 1.6 миллиона видов животных. И если у нас зоопарк со всеми видами животных, то вы с ума сойдёте разбираться в группировках или в отдельных видах.
Определять объект через наследование таким образом просто плохо. Видов банально средневекового оружия (включая Японское) больше 100 штук. И это неизбежно приведёт к забавным костылям. Что то, что называется моргенштерном по типу является "молотом". Как скажем в World Of Warcraft почти все нейтральные юниты являлись паладинами. Белка паладин конечно я уверен, что существует:
Но это не лучшее решение на уровне проектировки. А тут специально введено ограничение, что это RPG. То есть там много оружия.
Помимо этого в RPG у нас урон наносит не только оружие. Оружие наносит не только физический урон, но и магический, или урон огнём. А ещё оружие может иметь разное поведение атаки, а может и одинаковое. АОЕ урон может стать бафом с доп. уроном. Есть резисты, бафы и т.п. Давайте же спроектируем что-то здравое.
Сначала был урон
В рпг (да и многих играх) урон бывает разный. Поэтому я бы делал его сразу классом, а не просто float значением. Просто по той причине, что у вас вводится механика резистов, вводится механика брони и магического и физического урона (мы проектируем около скайрим, а там всё это есть). И вы это очень быстро сделаете. Это мелкая, но удобная правка. Такая штука конечно же не нужна для каких-то мелких и простых игр. Или скажем для среднего шутера. Но для рпг, моб, стратегий это полезный подход.
Атака — это поведение
В чём разница между интерфейсом и абстрактным классом и почему не стоит вводить абстрактный класс Weapon?
Есть старый как мир пример с поведением открывать. Открывать двери может лом, ключ карта или же металлический ключ. Но они совершенно разные по природе. У них нет некоего объекта обобщения. Их обобщает единое поведение в контексте системы.
С оружием есть обобщающий объект — оружие. Но в рпг помимо этого урон могут наносить ловушки, заклинания и много чего ещё. Поэтому обобщающим с точки зрения атаки должен быть именно интерфейс. Это в будущем будет очень удобно.
Тоже самое с получением урона. Является ли разрушаемый стол — Unit, а белка — "паладином"? Нет. Белка и стол просто "могут получать урон", так же как игрок, противники и всё что вы захотите ещё. И это единственное, что им надо знать в контексте урона. Поэтому заведём второй интерфейс.
Мы научились наносить урон и получать урон. Что дальше?
Оружие атакует?
Вот мы завели IAttack, теперь наследуем от него кучу конкретных классов оружий и всё классно. Не совсем.
Оружие — это модель каркас. Оно просто хранит нужные нам параметры, но атаки удобнее группировать по типам их поведения. AOE или типа того. Вот пример конкретной реализации такой атаки.
public struct AOEAttack : IAttack{
private Vector3 _position;
private AOEAttackData _attackData;
public AOEAttack(Vector3 position, AOEAttackData data)
{
_position = position;
_attackData = data;
}
public void Attack(){
IEnumerator<IDamagable> damagables = WorldManager.GetDamagable(position, _attackData.Area);
foreach(var damagable in damagables){
damagable.TakeDamage(_attackData.Damage);
}
}
}Что такое AOEAttackData? Для атаки нам нужно хранить как-то параметры этой атаки.
public class AttackData{
public DamageData Damage;
public AttackType AttackType;
}
public class AOEAttackData : AttackData{
public float Area;
}
public enum AttackType{
AOE, //Hammer Damage
Splash //Sword Damage
}Enum для определение типов атаки удобен для сериализации, и у нас получается достаточно стройная модель данных. Но у нас всё ещё нет оружия.
public class Weapon{
public AttackData AttackData;
public int Sustain;
}В данном случае оружие будет каркасом для общей модели данных. В будущем с информацией о всех бафах на оружии и т.п. для удобной сериализации. Но пока бафов у нас нет, у нас просто два оружия наносящих урон. Создадим молот и попробуем нанести урон, чтобы проверить как всё работает.
Weapon hammer = new Weapon()
{
AttackData = new AOEAttackData()
{
AttackType = AttackType.AOE,
Damage = new DamageData()
{
Damage = 100,
Type = DamageType.Physical
},
Area = 100
}
};Данные для атаки молота готовы. Допустим пока у нас атакует только пользователь. Во-первых, нам нужно создать атаку.
public static class AttackManager
{
public static IAttack GetAttack(Vector3 position, AttackData data)
{
switch (data.AttackType)
{
case AttackType.AOE:
return data is AOEAttackData aoeData ? new AOEAttack(position, aoeData) : null;
case AttackType.Splash:
return data is SplashAttackData splashData ? new SplashAttack(position, splashData) : null;
}
return null;
}
}По сути тут фабрика, где мы будем связывать конкретику с логикой. Вообще через рефлексию это можно сделать элегантнее. Но решение не идеальное, вы можете его улучшить в комментариях или предложить своё. Тоже думаю полезное упражнение.
И дальше наконец-таки атакуем. Дадим пользователю в руки молот и нанесём урон куда-то на клавишу "А".
public class Player : MonoBehaviour
{
private Weapon _weapon;
private void Start()
{
_weapon = new Weapon()
{
AttackData = new AOEAttackData()
{
AttackType = AttackType.AOE,
Damage = new DamageData()
{
Damage = 100,
Type = DamageType.Physical
},
Area = 100
}
};
}
private void Update()
{
if (Input.GetKeyDown(KeyCode.A))
{
var attack = AttackManager.GetAttack(transform.position, _weapon.AttackData);
attack.Attack();
}
}
}И в целом задача решена. Полную схему я приведу в конце. Теперь по одному нюансу моего решения.
Почему атаки — это команды?
Очень часто важен порядок атак. Чтобы скажем понимать юнит жив или мёртв, нужно ли наносить ему урон, запускать какую-то анимацию и т.п. И в реальной боевой архитектуре атака не наносилась бы пользователем, а шла бы скажем в AttackManager и в очередь атак и потом резолвилась бы эта очередь. Так получается чуть больше явного контроля + удобства для отмены той же атаки если всё произошло в один такт и один кадр.
В заключении
В реальной игре ещё были бы всякие бафы, глобал эффекты и т.п. и много чего ещё. Это чисто небольшой архитектурный срез одной подсистемы, которую в дальнейшем удобно расширять. На самом деле команды атаки и данные можно через рефлексию подтягивать в систему через то, что вы создаёте нужный вам тип атаки, и он дальше уже интегрирован во всю систему. Без добавления доп. строчки с as в AttackManager. И это может кто-то написать самостоятельно. Такая доп. задачка к задаче.
Финальная архитектура, которую у каджита уже можно было бы купить выглядит как-то так.
Мы сохранили заявленную изначальную функциональность. Не добавили ничего лишнего. Но при этом архитектура стала в разы удобнее с точки зрения её дальнейшего расширения. Если вам нравится игровая разработка и такие задачки, то подписывайтесь на мой блог! Там выходит много интересного.
Комментарии (31)
BorislavS
14.01.2023 16:59Очень информативно, спасибо! Преподаю unity на начальном уровне для старта и уже хочется самому глубже залезать в архитектуру, очень хорошо раскрываете суть)
Только ссылки надо поправить, кажется, они ведут не в тг, а на ютуб юнити
DyadichenkoGA Автор
14.01.2023 17:00Да, спасибо. Исправил. Видимо с утра запутался в своих ссылках и с поста забрал. Но видос там был тоже интересный :)
Slavenin999
14.01.2023 17:06Неа. Обе ссылки на ютуб...
DyadichenkoGA Автор
14.01.2023 17:07Это видимо кеш. У меня уже ок, ща протестил. Но если у кого вдруг ещё так лагает, то вот отдельно https://t.me/+ZypFCP_rxic4NTEy
GaricT
14.01.2023 17:17Итерацию с использованием ECS ждать?
DyadichenkoGA Автор
14.01.2023 17:18Нет, я не пользуюсь ECS. И как следствие не буду писать о том, в чем я не разбираюсь.
supremestranger
14.01.2023 18:04Что если сделать AttackData структурой, чтобы избежать аллокаций в AttackManager, а значения полей получать через геттер в интерфейсе?
DyadichenkoGA Автор
14.01.2023 18:14Да, норм улучшение. Но есть два нюанса.
Первое, что такие маленькие объекты пойдут в малую кучу, поэтому это не так критично. Юнити вроде как исправили то, что малой кучи в юнити не было.
А во вторых в шарпе нет наследования структур. Поэтому возникает вопрос «как расширять число полей». Композиция интерфейсов и адаптер. Ну может быть, но не уверен, что это будет сильно лучше.
Плюс основной вопрос. А зачем это оптимизировать? AOEAttack — это структура. Так как её аллоцировать мы будем часто. А AttackData чаще всего на запуске приложения и всё. Так как это данные оружия. Или при выпадении предмета забирая данные из базы. Или в момент загрузки юзера, чтобы посмотреть инвентарь. То есть вряд ли тут аллокации будут когда-то создавать фризы.
И жертвовать ради этого удобным расширением числа полей — не уверен, что стоит.
supremestranger
14.01.2023 18:16Действительно, в AttackManager аллоцируются структуры, тогда да, согласен.
GallopingDino
15.01.2023 20:34+1AOEAttack сразу боксится в IAttack, поэтому никаких преимуществ от того, что это структура мы не получаем.
michael_v89
14.01.2023 19:08Мне кажется, лучше делать метод Attack не в классе атаки, а в WorldManager. В реальности есть физические законы взаимодействия материи, оружие их использует, чтобы нанести урон. В игре нам не нужно детализировать WorldManager до такой степени, поэтому мы можем задать там более высокоуровневые законы взаимодействия (огонь, магию и т.д.), но это все равно часть WorldManager, он должен о них знать. Так не будет глобальной переменной в классе атаки, и WorldManager может ее модифицировать в зависимости от расстояния, или создавать новые атаки, например при выстреле в взрывающийся объект.
gandjustas
15.01.2023 01:20+1Не очень у вас ООП получилось.
Сначала:
_weapon = new Weapon() { AttackData = new AOEAttackData() { AttackType = AttackType.AOE, } // остальное пропущено }Вы два раза указываете тип атаки - в new и в enum-поле. Нарушена инкапсуляция.
Далее вы делаете:
var attack = AttackManager.GetAttack(transform.position, _weapon.AttackData); public static IAttack GetAttack(Vector3 position, AttackData data) { switch (data.AttackType) { case AttackType.AOE: return data is AOEAttackData aoeData ? new AOEAttack(position, aoeData) : null; case AttackType.Splash: return data is SplashAttackData splashData ? new SplashAttack(position, splashData) : null; } }Это вообще жесть, вы проверяете и enum-свойство и тип AttackData. Хотя достаточно одной проверки, но из-за нарушения инкапсуляции выше, получилось две. Да и в целом нарушает OCP (O в SOLID)
По хорошему надо было сделать полиморфный метод
GetAttackвAttackDataи писать так:var attack = _weapon.AttackData.GetAttack(transform.position)или еще лучше, чтобы не нарушать Закон Деметры (D в SOLID):
var attack = _weapon.GetAttack(transform.position)Внутри метода
Attackобращение кWorldManagerвыглядит крайне плохо, хреново тестируется и нарушает SRP и ISP (SOLID). По хорошему стоило бы написать так:private void Update() { if (Input.GetKeyDown(KeyCode.A)) { var attack = _weapon.AttackData.GetAttack(); // _worldManager нужного типа подсовывается в Player через DI attack.Attack(_worldManager, transform.position); } }А в идеале поделить один Attack на два метода - получение списка целей и нанесение урона.
Выглядело бы как-то так:
private void Update() { if (Input.GetKeyDown(KeyCode.A)) { var attack = _weapon.AttackData.GetAttack(); // _worldManager нужного типа подсовывается в Player через DI var damagables = _worldManager.GetDamagable(transform.position, attack); foreach(var damagable in damagables) { damagable.TakeDamage(_weapon.AttackData.Damage); } } } // Делаеим Double Dispatch public class WorldManager: IWorldManager { IEnumerator<IDamagable> GetDamagable(Vector3 position, IAttack attack) => attack.GetDamagable(this, transform.position) } // Кстати struct вам только помешает, ибо вы обращаетесь к нему через интерфейс public struct AOEAttack : IAttack { private AOEAttackData _attackData; public AOEAttack(AOEAttackData data) { _attackData = data; } public void Attack(IWorldManager worldManager, Vector3 position) => WorldManager.GetDamagable(position, _attackData.Area); }Пока написал понял что деление на
XXXAttackиXXXAttackDataне нужно. В целом у вас только одна точка где вам нужен полиморфизм -GetDamagable.Для полиморфизма в одном месте вам не надо городить две параллельные иерархии. Параллельные иерархии это вообще code smell.DyadichenkoGA Автор
15.01.2023 01:46+1Итак. Спасибо за ответ, всегда интересно послушать альтернативное мнение, но пройдёмся :)
Инкапсуляция — это в контексте задачи про композицию не важно.
Двойные типы. Да согласен. Это лишнее.
Двойная проверка в фабрике. Да тут и вся эта фабрика лишняя. Она мне нужна была для примера связывания в композиции.
А вот теперь про GetAttack в AttackData и почему нет. AttackData это модель, по хорошему она не должна определять поведение. Потому что много что может не зависеть от позиции и т.п. Хотя у вас она зависит, а потом резко не зависит.
Плюс это всё выглядит красиво до реальной реализации. Так как дальше появляется ворлд менеджер, который принимает на вход атаку. И мы получаем проблему супер жирного контроллера. Так как менеджер мира знает, как собрать данные о целях для конкретной атаки. И теперь у нас игрок собирает цели и атакует их. Это не совсем верная логика системы. И у нас SRP нарушает менеджер мира.
В моей же системе менеджер не знает ничего про атаку. Он забирает просто данные по площади в позиции не зная о таком понятии, как атака.
И логика этого проста. Атака сама реализует логику своей обработки, так как это её ответственность.
Внутри метода
Attackобращение кWorldManagerвыглядит крайне плохо, хреново тестируется и нарушает SRP и ISP (SOLID).То есть это чушь. Так как вызов WorldManager в атаке — не нарушает сингл респонсибилити. Так как наш запрос не меняет никакого состояния волрд менеджера. У нас есть стейт, мы из него запрашиваем данные для реализации нашей функциональности. Вот и всё. Там не усложняется никак тестирование. Так как волд менеджеру вообще должно плевать кто его вызывает. Он не в курсе. Если про тестирование атаки, чтобы подсунуть в неё мок менеджера. А для чего и зачем? В миниатюрный класс в 10 строк пихать юнит тест. Без мок мира целиком (то есть не интеграционный). А для чего?
Передавать его в качестве параметра метода тоже нет никакого смысла. Для сингл таргет цели мы передадим в конструктор нашей атаки конкретную цель допустим. И это будет в разы удобнее.
Последний блок кода в целом странный, но мне кажется вы просто запутались. Так как там что-то странное в AOEAttack.
Пока написал понял что деление на
XXXAttackиXXXAttackDataне нужно. В целом у вас только одна точка где вам нужен полиморфизм -GetDamagable.Для полиморфизма в одном месте вам не надо городить две параллельные иерархии. Параллельные иерархии это вообще code smell.Оно не обязательно было бы без оптимизаций и специфики. И вопрос подхода к архитектуре. Структура тут нужна для оптимизации. Так как атаки — это часто аллоцируемые команды. Которые поэтому нехорошо делать в виде классов. А данные (а точнее поля) расширять без классов неудобно. Это по сути про отличие объектов поведения, и объектов данных.
Можно было бы определять в XXXAttackData без XXXAttack без этой оптимизации. Если бы не ещё одно но. Разные поведения могут опираться на общие данные. Это в конкретной реализации получаются параллельные иерархии. А в реальной игре иерархия данных вполне может отличаться от иерархии объектов поведения. Опять-таки для удобства.
gandjustas
15.01.2023 02:23+1Инкапсуляция — это в контексте задачи про композицию не важно.
Слишком громкое заявление. Если вы нарушаете инкапсуляцию, то радо или поздно столкнетесь с проблемами. Это как гигиена. Можно раз не почистить зубы, но если так делать постоянно, то зубов у вас не останется довольно быстро.
Так как вызов WorldManager в атаке — не нарушает сингл респонсибилити. Так как наш запрос не меняет никакого состояния волрд менеджера.
Если про тестирование атаки, чтобы подсунуть в неё мок менеджера. А для чего и зачем? В минитюрный класс в 10 строк пихать юнит тест. Без мок мира целиком (то есть не интеграционный). А для чего?
Взаимоисключающие параграфы.
Дело не в том зачем писать тест. Дело в том что если тест написать легко, то как минимум принципы SOLID не нарушены. Если нет, то скорее всего нарушены.
В целом нет никаких формальных доказательств что SOLID делает архитектуру лучше, но эмпирические очень даже есть.
Из академического интереса напишите тест для реализаций IAttack, увидите кто чего нарушает.
Структура тут нужна для оптимизации. Так как атаки — это часто аллоцируемые команды.
Получая ссылку на интерфейс структуры вы делаете боксинг структуры. В курсе? Боксинг это аллокация и копирование данных из стека в кучу с последующем удалением сборщиком мусора.
Вы занимаетесь преждевременной оптимизацией, причем неудачно.
И вопрос подхода к архитектуре.
К сожалению ваш подход работает хуже, чем мог бы.
----------------------------------------
Это об было архитектуре вообще. А теперь частности:
AttackData это модель, по хорошему она не должна определять поведение.
Честно говоря не пойму кто кому чего должен. Вы сначала придумали классы и объекты, а потом пытаетесь к ним прикрутить какое-то поведение или не прикрутить. Не надо так делать.
Так как дальше появляется волд менеджер, который принимает на вход атаку. И мы получаем проблему супер жирного контроллера.
Метод можно сделать Extension_ом и все будет хорошо. Это я понял уже после того как написал.
public static IEnumerator<IDamagable> GetDamagable(this IWorldManager manager, Vector3 position, IAttack attack) => manager.GetDamagable(this, transform.position)Остальной код не поменяется.
Это не совсем верная логика системы.
Это субъективное заявление. Просто логика мира в вашей голове отличается от логики в других головах. Помните всегда об этом.
И у нас SRP нарушает менеджер мира.
Он и так будет SRP нарушать. Это фасад к куче разных подсистем. По хорошему надо его разделить на эти самые подсистемы и инжектить конкретные подсистемы в конкретные классы. Но так снижается discoverability. Кроме того подсистемы связаны между собой, тогда разделение может нанести вреда больше чем пользы.
DyadichenkoGA Автор
15.01.2023 02:39Про SRP да согласен тупанул. Там же ответственность не инкапсулирована целиком в класс.
Он и так будет SRP нарушать. Это фасад к куче разных подсистем.
Ну в моей идее не совсем, так как тут плохое название. Но нет, он не будет фассадом, так как допустим в сингл таргете у нас вообще не будет такого менеджера. И если лоцировать, то через конструктор команды.
А так спасибо за дополнения.
gandjustas
15.01.2023 01:41+2Выше я объяснил что не так в вашем коде, а теперь попробую донести ошибки мышления, которые привели к ошибкам в коде:
Атака — это поведение
Нет, нет и еще раз нет. Я не смогу объяснить это лучше, чем сделал Эрик Липперт в своей классической серии статей https://ericlippert.com/2015/04/27/wizards-and-warriors-part-one/ и далее. Когда-нибудь я переведу их для хабра.
Вкратце суть: не надо строить иерархии классов на основе объектов игрового мира (или того, что вы моделируете в своей программе).
Во-первых, нам нужно создать атаку.
...
По сути тут фабрика
Вы хотели применить паттерн (фабричный метод), но никаких оснований для его применения не было, потому что тип вам и так известен в этой точке программы, вы его явно задали выше. Фабричный метод нужен только тогда, когда вы НЕ знаете в данной точке программы какой тип вам нужен.
Почему атаки — это команды?
Потому что любая игра это цикл: анализ текущего состояния мира + анализ ввода -> генерация команд -> изменение состояния мира.
С этого надо было начинать.
Ключевая ошибка - проектирование снизу вверх. Вы сначала придумываете объекты игрового мира, поведение, а потом пытаетесь это все засунуть в правильную архитектуру.
Просто пойдите другим путем. Начните с простого цикла, как я описал выше. А потом декомпозируйте функции, заменяйте большие неуклюжие if и case на полиморфные методы, применяйте паттерны там где они уместны и у вас родится идеальная иерархия классов.
DyadichenkoGA Автор
15.01.2023 01:55Нет, нет и еще раз нет. Я не смогу объяснить это лучше, чем сделал Эрик Липперт в своей классической серии статей https://ericlippert.com/2015/04/27/wizards-and-warriors-part-one/ и далее. Когда-нибудь я переведу их для хабра.
Стоп-стоп-стоп. Эрик Липперт говорит не про атаку. И то что это поведение никаким образом не относится к его заметке. И эту проблему как раз и решает композиция. То что описано на пол статьи, почему не надо наследовать оружие и т.п. Мышление идёт нет не от объектов игрового мира, а от процессов. Абстрактное проектирование вундерфавли я в целом не приветствую. Но это другой вопрос. Просто эта заметка вообще не про то.
Идеальной архитектуры не существуют. Давайте поговорим теперь в продуктово-бизнесовом ключе. Какие недостатки у этой архитектуры объективные? Риски и проблемы? Долгосрочные. В контексте реальной реализации. Я просто поэтому не люблю все архитектурные споры, особенно с адептами солида, так как время тратящееся на соблюдение всех мантр SOLID в среднем проекте просто не окупается.
Но изначально, я не строил идеальную архитектуру. Я просто составил задачу на композицию и решил её. Лучше, чем была изначальная схема. Вот и всё.
DyadichenkoGA Автор
15.01.2023 02:20Важно наверное сделать ремарку. А то может мой ответ может звучать как-то не так. И кто-то обижается на "адептов солида". Я ни в коем случае не принижаю вашу экспертизу. Я думаю, что вы так же, как и я сделали уже больше 100 проектов за карьеру.
Я просто не люблю разбор "что не так по солиду". Так как мне столько историй рассказывают про синглтоны те же. Что это смертный грех. Но за 100+ проектов (и три крупных с продакшенами по несколько лет и большими командами) я их там видел. И они не ломаются каждые 5 минут, как предсказывали "свидетели Солид". Я просто не люблю аргументацию "нарушается такой-то принцип солид".
Аргумент с тестированием скажем хороший, но в реальном контексте системы несущественный. Так как такой класс нет никакого смысла покрывать юнит тестом. Если очень хочется, можно выкинуть в конструктор присвоение менеджера. Можно вообще не знать о менеджере и передавать массив юнитов. Там можно так глубоко уходить в "а можно вот так, почему атака знает о каком-то там менеджере, если ей нужны юниты", "а если у нас специфика юнитов, то зачем нам разные типы атаки, мы же всегда можем передавать массив юнитов". И крутить этот кубик-рубика в любую сторону. Находя аргументацию любого из подходов.
Я же люблю говорить о бизнесовых и продуктовых недостатках. В сайд эффектах таких решений. Например дублирование иерархии. Нам придётся писать код в трёх местах, чтобы добавить фичу. Это плохо. Это лишний контракт. Его можно избежать. Но когда это 40 строк кода — это не крит. И тут не надо уходить в крайности главное, хотя велик соблазн. И я когда-то так делал в таких спорах. "Так про всё можно сказать в таком случае". Нет нельзя. Так как есть логика системы и понимание из опыта, что возможно, а что нет.
gandjustas
15.01.2023 02:39Раз вы за бизнес-подход, то вы наверное понимаете, что есть формальные (независимые от мнения) метрики: количество строк кода, количество классов, цикломатическая сложность, связность классов. Чем выше эти метрики, тем дороже стоит написание и поддержка кода (последующее внесение изменений в код).
Если эмпирические данные, что соблюдение SOLID в целом снижает эти метрики. Это не вопрос нравится SOLID или нет.
Понятно что в коде на 40 строк все это не нужно. Но вы этот код пишите для того, чтобы нам показать как надо, в том числе когда кода станет не 40 строк, а 40к.
DyadichenkoGA Автор
15.01.2023 02:50Да, но там не будет 40к. Типов атаки ну 20 придумать сложно даже. Просто если мы начнём это обсуждать и дискутировать, то я уверен, что мы дойдём, что атака вообще должна быть одна. Так как она принимает таргеты и всё. И это будет лучше. Какая разница как они определяются атаке? А определение таргетов должно быть в другом месте. И может для сериализации удобнее будет енам, который идёт куда-то. Тут немного другой контекст наверное.
То есть скажем так. Ваше дополнение ценно. И моя эмоциональная реакция неверная. Минусы задевают самолюбие, как и критика. Особенно исходя из того как хабр ранжирует статьи. И я постепенно учусь это гасить (вот уже 40 статей).
Просто суть статьи для меня немного не в этом. Да и суть задачи. Задача была исправить изначально кривую архитектуру на лучше. Так как формулировать задачи и их решения большой блок работы. И я считаю эту задачу хорошим упражнением.
Как говорится, я как и любой человек не идеален. И то, что статья порождает дискуссии это хорошо.
Но на тему бизнес-метрик, цикломатическая сложность, поддержка кода и т.п. Вот это уже зависит от масштаба разработки и сроков проектировки этого дела. Тут есть нюанс и контекст. Абстрактная архитектура дольше проектируется и дороже. И то что время на проектировку фичи, которая выдержит расширение на 40к строк, при том что потолок тут 1к строк теоретический не факт, что окупит затраты на поддержку. И не факт, что затраты на поддержку будут ниже, чем затраты на проектировку и разработку. И я говорил про кубик-рубика в этом контексте скорее. То есть мы можем потратить месяц на проектировку и рефакторинг к идеалу. А можем написать реализацию за один день и не встретить проблем.
И условно на мой взгляд есть уровень проектировки удовлетворяющий условиям задачи, чтобы не уходить в оверинжиниринг. Так как доводить код до совершенства может быть очень долго, но не выгодно с точки зрения сроков реализации какой-либо фичи. То есть ваше вполне корректное расширение на базе моего решения может быть верным. Но вы потратили время поверх моего времени и так далее.
gandjustas
15.01.2023 03:49+1Да, но там не будет 40к. Типов атаки ну 20 придумать сложно даже.
В программировании есть три числа 0, 1 и N. Все N равнозначны. Если вы показываете пример как надо для N=2, то он должен работать для любого N (по крайней мере в разумных пределах).
Но на тему бизнес-метрик, цикломатическая сложность, поддержка кода и т.п. Вот это уже зависит от масштаба разработки и сроков проектировки этого дела.
Смотрите выше замечание про N. Нам неинтересно как вы решаете задачу для N=2, покажите нам способ решения для любого разумного N, особенно с учетом того, что N будет меняться в процессе разработки и поддержки.
И то что время на проектировку фичи, которая выдержит расширение на 40к строк, при том что потолок тут 1к строк теоретический не факт, что окупит затраты на поддержку.
Еще раз посмотрите на первое замечание про N.
Задача была исправить изначально кривую архитектуру на лучше.
Увы предыдущую архитектуру не видел, но эта статья показывает что у вас еще есть возможность сделать лучше.
Абстрактная архитектура дольше проектируется и дороже
Так не занимайтесь этим. Начните писать код без наследования и полиморфизма вообще. Прям в методе
Updateпишите.Если вдруг метод перестанет умещаться на экране редактора кода, то сделайте Extract method.
Если код дублируется, то тоже сделайте Extract method.
-
Если у вас много функций с одними и теми же параметрами, то перенесите их в отдельный класс.
Потом сделайте Extract Interface и ссылайтесь только на интерфейс.
Когда вам второй раз повстречается
switch(attackType)сделайте интерфейсIAttackи несколько реализаций вместо этогоswitch.
Думаю суть вы поняли.
И условно на мой взгляд есть уровень проектировки удовлетворяющий условиям задачи, чтобы не уходить в оверинжиниринг.
Конечно есть, я выше описал как этого уровня достичь и не промахнуться.
DyadichenkoGA Автор
15.01.2023 03:52Увы предыдущую архитектуру не видел, но эта статья показывает что у вас еще есть возможность сделать лучше.
Первый скрин в статье.
Так не занимайтесь этим. Начните писать код без наследования и полиморфизма вообще. Прям в методе
Updateпишите.Это уже уход в крайность и поэтому неинтересно обсуждать.
В программировании есть три числа 0, 1 и N. Все N равнозначны. Если вы показываете пример как надо для N=2, то он должен работать для любого N (по крайней мере в разумных пределах).
Не согласен с аналогией. Так как у нас Nmax определено выше.
gandjustas
15.01.2023 05:03Первый скрин в статье.
Диаграмма классов не имеет никакого смысла без примера кода их использования. Именно использование вашей иерархии классов может показать хорошая архитектура или нет.
Это уже уход в крайность и поэтому неинтересно обсуждать.
Это путь познания. Сделайте так один раз и вы будете понимать в ООП гораздо лучше других.
Не согласен с аналогией. Так как у нас Nmax определено выше.
Это у Вас (автора) определено. Нам (читателям) не интересен ваш Nmax. Мой Nmax может оказаться в 1000 раз больше вашего, подойдет ли мне ваш подход?
gandjustas
15.01.2023 02:25Видимо вы еще не прочитали до конца весь цикл статей Липперта (их там семь штук). А если успели прочитать за менее чем за 15 минут, то вряд ли успели осознать.
DyadichenkoGA Автор
15.01.2023 02:30Я когда-то давно читал. Может стоит перечитать конечно. Но он в цикле статей разбирал проблему, которая решается так же через композицию. Условно с тезисом, что не надо систему писать так, как кажется логичным. И "прямым ООП". И относится к ней более абстрактно.
Tr0sT
15.01.2023 12:29По-моему первая картинка выглядит как результат рефакторинга последней :) Функционал идентичный, но разница в простоте огромная.
Судя по статье - какие варианты расширения предполагает автор? Добавление новых типов оружия. В начальном случае для добавления нового типа оружия нужно добавить... новый наследник Weapon. Звучит просто и логично.
А во втором? Новые наследники IAttack и AttackData, да ещё не забыть добавить кейс в AttackManager.GetAttack
И почему тогда второй вариант гибче первого, если добавлять во втором приходится больше, и в несколько мест? Одна из задач архитектуры - локализовать точки расширяемости кода, и первый вариант справляется с этой задачей лучше.
Вообще архитектура, т.е. добавление слоёв абстракции - это увеличение сложности программы, и за счёт этого должны решаться какие-то проблемы, но в статье не показаны никакие проблемы первого варианта, поэтому все эти трансформации выглядят как оверинженеринг.
agalakhov
Вот в общем-то поэтому из Rust исключили наследование и понятие класса как таковое, а оставили только struct и trait. Программисты на С++/C#/Java называют это "интерфейс".