---
# System prepended metadata

title: Piggy Bank review

---

# Piggy Bank review

1. Лишняя ;
![](https://i.imgur.com/ua95hDD.png)
![](https://i.imgur.com/N6jowTL.png)
![](https://i.imgur.com/vFRzCTL.png)

2. Неявное преобразование типов
![](https://i.imgur.com/3RoXm07.png)
![](https://i.imgur.com/BeWMDCY.png)
![](https://i.imgur.com/co7Aqi3.png)

3. Лишние инклуды, файл path, файл без изменений
![](https://i.imgur.com/wvt7tzI.png)
![](https://i.imgur.com/xKbX3LU.png)
![](https://i.imgur.com/6pJEnWf.png)
![](https://i.imgur.com/VXUC2Qg.png)

4. Перенос из секции private в секцию protected родительского класса необходимо обговорить с автором, так как в этом закладывалась некоторая логика и было сделано не просто так. Это нарушение логики архитектуры. И я так понимаю, может повлечь в дальнейшем к багам, когда кто-то  будет переопределять эти методы. Лучше так не делать.
![](https://i.imgur.com/5nSUzyL.png)
![](https://i.imgur.com/K5jdH3W.png)


----------------------
5. Для чего
![](https://i.imgur.com/hTzvH1f.png)

6. Это, наверное, лишнее, так как нет Attach
![](https://i.imgur.com/LRV5ZlV.png)

7. RepeatedSpineAnimation - класс, который в перспективе можно переиспользовать. Единственное, вызов статических методов хелпера игры может препятствовать этому. Возможно, стоит подумать и типичные чистые методы, которые повторяются с игры в игру перенести куда-то в глобальное место gui::Utils. Название класса не соответствует сути, так как это не просто повторяющаяся спайн анимация, а повторяющаяся спайн анимация на барабанах.
![](https://i.imgur.com/XEMTSlk.png)

8. Есть AddTask - нет RemoveAllTasks при релизе на случай принудительного выхода
![](https://i.imgur.com/Moo4lfG.png)
![](https://i.imgur.com/kbbFzeu.png)
![](https://i.imgur.com/Sn8xFLR.png)
![](https://i.imgur.com/8RJA0mg.png)


9. Лучше тут использовать count, так как при проверке через квадратные скобки создается элемент при отсутствии элемента по ключу. Если по логике не задумано подобное. Ранее мы сталкивались с неприятными последствиями подобного в логере когда-то. По коду нужно просмотреть подобные моменты, много где встречается.
![](https://i.imgur.com/CfFyp5s.png)


10. Может, тут стоит удалять экшн со списка по его окончанию
![](https://i.imgur.com/qWWsqcY.png)

11. Нужно проверить кейсы имитации свертывания приложения на телефонах на данном моменте.В виндовом клиенте достаточно зажать шапку окна и подержать некоторое время его так. Такая логика выглядит очень сомнительно для данного кейса.
![](https://i.imgur.com/eJj4OMR.png)

12. Тут бы добавить схему связей моделей для более простого понимания как мы обычно делаем и небольшие комментарии что за что отвечает
![](https://i.imgur.com/4PsT3hU.png)

13. Странный тип записи с форвард декларейшн, в нашем проекте такого не встречала
![](https://i.imgur.com/lvADWkC.png)

14. Для PigletMergeAnimation я бы использовала композицию, так как этот класс слушает по-сути только LockItFeature. Это бы инкапсулировало логику и упростило общее понимание.
А LockItFeatureFinalAnimation - часть PigletMergeAnimation. Это на первый взгляд, может, что-то не видно.
Не ясно для чего связь
![](https://i.imgur.com/9MfRh6r.png)
