Как и обещал — делаю разбор кода с прошлой публикации. Поехали!
Ошибка 1. GameManager
Название класса не информативное. Можно очень легко понять хорошее ли название класса, метода или переменной. Для этого достаточно посмотреть на название и не изучая код понять, за что отвечает эта сущность. Название GameManager означает, что внутри этого класса может быть все что угодно.
Ошибка 2 и 3
public static event Action OnCubeSpawned = delegate { };
2. Сам класс не является статическим или синглтоном (то есть быть в единственном экземпляре), при этом эвент статический. Статический эвент только в статических классах
3. = delegate { };
Получается мы создаем пустой делегат и всегда его вызываем, что не имеет смысла. Тем более в C# принято вызывать эвенты через ?.Invoke()
Ошибка 4 и 5
private void Awake()
{
spawners = FindObjectsOfType<CubeSpawner>();
}
4. FindObjectsOfType<CubeSpawner>();
Ну тут наверное каждый разработчких схватился за сердце, когда увидел эту строчку. Во первых тут есть скрытая зависимость и нарушение принципа D (SOLID), во вторых это тяжелая операция
5. Еще одной ошибкой является вызов в Awake()
Не даром Unity сделала 2 метода инициализации: Awake и Start, и их условное различие в том, что Awake вызывается для инициализация внутренних компонентов скрипта, а Start для внешних. Если придерживаться этого правила, то можно будет избежать багов и проверок на инициализацию компонентов. Но лучше использовать DI
Ошибка 6, 7, 8
if (Input.GetButtonDown("Fire1"))
{
...
}
6. Лучше использовать конструкцию if-return
if (!Input.GetButtonDown("Fire1"))
return;
Тогда код не поменяет свою логику, а код станет читабельнее
7. "Fire1" стоит вынести в константу и использовать ее, вместо конкретной строки
8. Input это отдельная ответственность, что нарушает принцип S (SOLID), такую логику нужно вынести в отдельный класс и добавить эвент на конкретное действие
Ошибка 9
currentSpawner = spawners[UnityEngine.Random.Range(0,2)];
Вот тут вообще фатальная на мой взгляд ошибка. В логике кода сильная зависимость, что размер массива равен 2, А любое изменение в количестве кубов приведет на сцене к багу.
Ошибка 10
Общий код стиль проекта отличается от Microsoft code convention
Отсутствие private, название полей без "_", отсутствие отступов, но, когда я провожу ревью, я обычно отношу эти недостатки к несущественным, так как можно довольно легко их исправить через код стиль райдера, например. Исключением является, пожалуй совсем уж странные подходы к форматированию кода, такие как венгерская нотация или форматирование через Tab как в шейдерах
А первым подписчиком, который назвал большинство ошибок был @arper
В первом комментарии приложу пример рефакторинга данного кода, без изменения архитектуры
P.S Спасибо всем, кто проявил активность в прошлом посту! А я уже дописываю пост про софт скилы