Part 1 1. Потрібно прибрати перевизначення методів, де просто викликається метод базового класу (передивитися по коду) ![photo_2024-04-29_10-10-03](https://hackmd.io/_uploads/HJa-XAnZA.jpg) 2. Перенести ініціалізацію popup_visual_manager з PopupProxy::OnPrepare в сеттер, та виконувати це на рівні AppPlugInExpanded ![photo_2024-04-29_10-10-10](https://hackmd.io/_uploads/BkvM7R3bA.jpg) ![Screenshot_1](https://hackmd.io/_uploads/ryAzmC2W0.png) 3. Зв'язок моделі бонуски і попапу досить дивний, як на мен. Щоб не переносити поки бонуску в vsdk, да і взагалі трохи інкапсулювати можливість з попапу порушувати логіку роботи бонуски (так як доступні важливі публічні методи), пропоную стоврити інтерфейс з методом SetupBonus, та передавати бонуску в попап з обмеженим доступом. Інтерфейс буде в vsdk і нічого поки переносити не потрібно. В іншому випадку забагато буде змін для однієї задачі. 4. Дивна взаємодія абстрактного класу та класу, що його наслідує в BasePopup_Responder/BasePopup_Initiator (GetPresentModel). Чому вирішили саме так реалізувати? ---------------------------------------- Part 2 1. Тут потрібно було було передавати екзампляр класу, а не конфіг. Це не найкращий підхід ![Screenshot_6](https://hackmd.io/_uploads/Hyxd_cJM0.png) 2. Погана назва, назвою потрібно описати для чого цей інтерфейс взагалі ![Screenshot_7](https://hackmd.io/_uploads/SyhQYc1zR.png) 3. **BasePopup_Initiator** - Прибрати наслідування від vlib::VLIB_Model для BasePopup_InitiatorImpl, воно не потрібне. ![Screenshot_8](https://hackmd.io/_uploads/S1yIfjJzA.png) - Прибрати m_impl, він також не потрібний. - Навіщо цей callback, чому не викликати просто метод? ![Screenshot_9](https://hackmd.io/_uploads/ByC-vskMC.png) - m_additional_callback, SetModelCallback, m_callback_by_series - дивні назви. m_additional_callback - це CallNextModelCallback m_callback_by_series просто потрібно зробити методом - cpp::count_ptr<.vlib::VLIB_Model> m_present_model має бути в BasePopup_Initiator, і краще обмежити доступ інтерфейсом, який буде делегувати необхідний функціонал. virtual VLIB_Model* const GetModel(int identity) const; virtual VLIB_Model* const GetModel(const std::string& symbolic) const; virtual void CallExternal(vevt::VEVT_Model* model); - GetPresentModel прибрати, передавати через конструктор по визначеному вище інтерфейсу **BasePopup_Responder** Аналогічні зауваження 4. - BasePopup_LockItFeature не використовує функціонал BasePopup_Initiator? ![Screenshot_10](https://hackmd.io/_uploads/r1yvW3kfA.png) - BigWin_SpineEdition не використовує функціонал BasePopup_Responder? Для усунення неоднозначності і більшої наглядності, пропоную BasePopup_Responder і BasePopup_Responder не наслідувати від BasicPopupModel, а використати асоціацію :), тобто зробити навпаки. Зробити клас BasicPopupModelResponder, який буде наслідувати BasicPopupModel і містити екземпляр PopupResponder, який буде ініціалізуватися через конструктор, тобто, якщо потрібно буде змінити логіку Responder, ми зможемо це зробити зовнішньо, в ідеалі в AppPlugin. Аналогічно для BasePopup_Initiator.