# 434 code review

- 
Сложное для понимания сути название метода. Возможно, стоит заменить TravelEffect на StartBonusEffect, а уже сам эффект можно назвать Travel..
- 
я бы убрала дефолтные значения такого типа, очень неявно, пусть пользователь осознанно пишет рандомизировать или нет, тем более по дефолту true

- 
для чего тут 2 конструктора?

- 
такие штуки лучше через наследование с общим интерфейсом, а не просто добавлять новый параметр, который переопределяет поведение

- SquigglyMainVisual - сложно читать, а особенно сложно будет произносить в объяснении это кому-то еще :) Может стоит выбрать название попроще.
- в чем нужда добавления этого параметра?
- почему бы не использовать наши указатели чтобы не разрознять логику
- в операциях с игровыми объектами лучше проверять существуют ли они и случайно ли не на стадии разрушения, помню бывали фантомные краши и часто причиной были именно такие обращения без проверок
вот пример такой проверки, может, стоит вынести выше на уровень и тогда не думать уже об этом в хендлерах
Тут тоже может быть краш 
- почему Billiard ?
- полезно прицельно иногда писать небольшие комментарии, например, в таких случаях, это помогает в понимании неявных моментов
- 
лучше это выделить через константы для облегчения чтения
- файл можно удалить
-
- Это нужно отдельным коммитом не забыть
- это мердж ?
- для чего еще 1 метод StartBonus? стоит описать комментарием
- лишние инклуды

**SweetKingBonus**
- как показывает опыт, такие штуки
лучше сразу выносить в структурку-конфиг бонуски
вместе с такими параметрами
и потом уже обращаться к полям структуры + удобно настроить при переиспользовании
- может, это стоит вынести как визуальный эффект и не смешивать логику
- для чего m_pop_sound_ids? можно использовать m_active_sound_actions_id через метод AbstractIndependentBonus::PlaySound,
- важно перепроверить суммы выплат на разных ставках, что нам присылает сервер и учитывается ли там уровень ставки

**Drill**
- класс DrillFireComponent стоит перенести в фильтр компонентов
- дрель была реализована так, что обработка визуальных эффектов (айдл анимаций, выстрела) была заложена в оружии, для Кенди ты перенес часть логики в файер компонент, это усложняет понимание

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

- странная запись, нужно разделить. Какая идея статической приватной переменной?