---
# System prepended metadata

title: 434 code review

---

# 434 code review

- ![](https://i.imgur.com/m6k3Ixc.png)
Сложное для понимания сути название метода. Возможно, стоит заменить TravelEffect на StartBonusEffect, а уже сам эффект можно назвать Travel..
- ![](https://i.imgur.com/63bIWVU.png)
я бы убрала дефолтные значения такого типа, очень неявно, пусть пользователь осознанно пишет рандомизировать или нет, тем более по дефолту true

- ![](https://i.imgur.com/UTMEWqq.png)
для чего тут 2 конструктора?

- ![](https://i.imgur.com/Qbm84vU.png)
такие штуки лучше через наследование с общим интерфейсом, а не просто добавлять новый параметр, который переопределяет поведение

-  SquigglyMainVisual - сложно читать, а особенно сложно будет произносить в объяснении это кому-то еще :) Может стоит выбрать название попроще.
-  в чем нужда добавления этого параметра?
  ![](https://i.imgur.com/H5y5Lt7.png)
- почему бы не использовать наши указатели чтобы не разрознять логику
![](https://i.imgur.com/USk3KSH.png)
- в операциях с игровыми объектами лучше проверять существуют ли они и случайно ли не на стадии разрушения, помню бывали фантомные краши и часто причиной были именно такие обращения без проверок
![](https://i.imgur.com/qbB6KYl.png)
вот пример такой проверки, может, стоит вынести выше на уровень и тогда не думать уже об этом в хендлерах![](https://i.imgur.com/0tPOOlk.png)
Тут тоже может быть краш ![](https://i.imgur.com/IIWH00h.png)
- почему Billiard ?
![](https://i.imgur.com/hdnyurh.png)
- полезно прицельно иногда писать небольшие комментарии, например, в таких случаях, это помогает в понимании неявных моментов
 ![](https://i.imgur.com/DbTFHTy.png)
- ![](https://i.imgur.com/zvHkmE3.png)
![](https://i.imgur.com/wCKIMZh.png)
лучше это выделить через константы для облегчения чтения
- файл можно удалить
![](https://i.imgur.com/8xh2Bfj.png)
- 
- Это нужно отдельным коммитом не забыть 
 ![](https://i.imgur.com/PBFRgJd.png)
- это мердж ?
![](https://i.imgur.com/5mIXemT.png)
- для чего еще 1 метод StartBonus? стоит описать комментарием
![](https://i.imgur.com/uSN4XMn.png)
- лишние инклуды
![](https://i.imgur.com/ESHGEnF.png)
![](https://i.imgur.com/K7aW22k.png)
![](https://i.imgur.com/vbztaaH.png)
![](https://i.imgur.com/Kp7cWYd.png)
![](https://i.imgur.com/020tUF2.png)
![](https://i.imgur.com/xbahQc1.png)

**SweetKingBonus**
- как показывает опыт, такие штуки 
![](https://i.imgur.com/qgHJi08.png)
лучше сразу выносить в структурку-конфиг бонуски
![](https://i.imgur.com/sdcErhu.png)
вместе с такими параметрами
![](https://i.imgur.com/rgKcyaV.png)
и потом уже обращаться к полям структуры + удобно настроить при переиспользовании
- может, это стоит вынести как визуальный эффект и не смешивать логику
![](https://i.imgur.com/JwzdT3e.png)
- для чего m_pop_sound_ids? можно использовать m_active_sound_actions_id через метод AbstractIndependentBonus::PlaySound,
- важно перепроверить суммы выплат на разных ставках, что нам присылает сервер и учитывается ли там уровень ставки

**Drill**
- класс DrillFireComponent стоит перенести в фильтр компонентов
- дрель была реализована так, что обработка визуальных эффектов (айдл анимаций, выстрела) была заложена в оружии, для Кенди ты перенес часть логики в файер компонент, это усложняет понимание
![](https://i.imgur.com/2gigQS1.png)

**HonneyBarrel**
- HoneyBarrelFireComponent - я бы назвала как-то универсально MultiExplosiveBarrelsFireComponent + перенесла в базовый набор компонентов, там же все унифицировано и готово к переиспользованию, только поправить логи и название в конфиге "honey_barrels"


- странная запись, нужно разделить. Какая идея статической приватной переменной?
![](https://i.imgur.com/ufl91VH.png)



