From dd15c6d90fc30955141faa37c027123f49ec13a3 Mon Sep 17 00:00:00 2001 From: Daria Volvenkova Date: Mon, 11 Dec 2017 20:34:37 +0300 Subject: [PATCH] Review fixes. --- map/bookmark_manager.cpp | 54 +++++++++----------- map/bookmark_manager.hpp | 45 ++++++++++------- map/framework.cpp | 26 +++++----- map/map_tests/bookmarks_test.cpp | 87 +++++++++++++++++++++++++++++++- map/user_mark_container.cpp | 11 ++-- map/user_mark_container.hpp | 3 +- 6 files changed, 154 insertions(+), 72 deletions(-) diff --git a/map/bookmark_manager.cpp b/map/bookmark_manager.cpp index 261f59d0e0..0b970d4602 100644 --- a/map/bookmark_manager.cpp +++ b/map/bookmark_manager.cpp @@ -27,6 +27,8 @@ #include +using namespace std::placeholders; + namespace { char const * BOOKMARK_CATEGORY = "LastBookmarkCategory"; @@ -64,23 +66,14 @@ std::string const GenerateValidAndUniqueFilePathForKML(std::string const & fileN } } // namespace -BookmarkManager::BookmarkManager(GetStringsBundleFn && getStringsBundleFn, - CreatedBookmarksCallback && createdBookmarksCallback, - UpdatedBookmarksCallback && updatedBookmarksCallback, - DeletedBookmarksCallback && deletedBookmarksCallback) - : m_getStringsBundle(std::move(getStringsBundleFn)) - , m_createdBookmarksCallback(std::move(createdBookmarksCallback)) - , m_updatedBookmarksCallback(std::move(updatedBookmarksCallback)) - , m_deletedBookmarksCallback(std::move(deletedBookmarksCallback)) - , m_bookmarksListeners(std::bind(&BookmarkManager::OnCreateUserMarks, this, - std::placeholders::_1, std::placeholders::_2), - std::bind(&BookmarkManager::OnUpdateUserMarks, this, - std::placeholders::_1, std::placeholders::_2), - std::bind(&BookmarkManager::OnDeleteUserMarks, this, - std::placeholders::_1, std::placeholders::_2)) +BookmarkManager::BookmarkManager(Callbacks && callbacks) + : m_callbacks(std::move(callbacks)) + , m_bookmarksListeners(std::bind(&BookmarkManager::OnCreateUserMarks, this, _1, _2), + std::bind(&BookmarkManager::OnUpdateUserMarks, this, _1, _2), + std::bind(&BookmarkManager::OnDeleteUserMarks, this, _1, _2)) , m_needTeardown(false) { - ASSERT(m_getStringsBundle != nullptr, ()); + ASSERT(m_callbacks.m_getStringsBundle != nullptr, ()); m_userMarkLayers.emplace_back(my::make_unique()); m_userMarkLayers.emplace_back(my::make_unique()); @@ -382,7 +375,7 @@ size_t BookmarkManager::LastEditedBMCategory() } if (m_categories.empty()) - CreateBmCategory(m_getStringsBundle().GetString("my_places")); + CreateBmCategory(m_callbacks.m_getStringsBundle().GetString("my_places")); return 0; } @@ -397,52 +390,53 @@ BookmarkCategory * BookmarkManager::GetBmCategory(size_t index) const return (index < m_categories.size() ? m_categories[index].get() : 0); } -void BookmarkManager::OnCreateUserMarks(UserMarkContainer * container, df::IDCollection const & markIds) +void BookmarkManager::OnCreateUserMarks(UserMarkContainer const & container, df::IDCollection const & markIds) { - if (container->GetType() != UserMark::Type::BOOKMARK) + if (container.GetType() != UserMark::Type::BOOKMARK) return; - if (m_createdBookmarksCallback == nullptr) + if (m_callbacks.m_createdBookmarksCallback == nullptr) return; std::vector> marksInfo; GetBookmarksData(container, markIds, marksInfo); - m_createdBookmarksCallback(marksInfo); + m_callbacks.m_createdBookmarksCallback(marksInfo); } -void BookmarkManager::OnUpdateUserMarks(UserMarkContainer * container, df::IDCollection const & markIds) +void BookmarkManager::OnUpdateUserMarks(UserMarkContainer const & container, df::IDCollection const & markIds) { - if (container->GetType() != UserMark::Type::BOOKMARK) + if (container.GetType() != UserMark::Type::BOOKMARK) return; - if (m_updatedBookmarksCallback == nullptr) + if (m_callbacks.m_updatedBookmarksCallback == nullptr) return; std::vector> marksInfo; GetBookmarksData(container, markIds, marksInfo); - m_updatedBookmarksCallback(marksInfo); + m_callbacks.m_updatedBookmarksCallback(marksInfo); } -void BookmarkManager::OnDeleteUserMarks(UserMarkContainer * container, df::IDCollection const & markIds) +void BookmarkManager::OnDeleteUserMarks(UserMarkContainer const & container, df::IDCollection const & markIds) { - if (container->GetType() != UserMark::Type::BOOKMARK) + if (container.GetType() != UserMark::Type::BOOKMARK) return; - if (m_deletedBookmarksCallback == nullptr) + if (m_callbacks.m_deletedBookmarksCallback == nullptr) return; - m_deletedBookmarksCallback(markIds); + m_callbacks.m_deletedBookmarksCallback(markIds); } -void BookmarkManager::GetBookmarksData(UserMarkContainer * container, df::IDCollection const & markIds, +void BookmarkManager::GetBookmarksData(UserMarkContainer const & container, df::IDCollection const & markIds, std::vector> & data) const { + data.clear(); data.reserve(markIds.size()); for (auto markId : markIds) { - auto const userMark = container->GetUserMarkById(markId); + auto const userMark = container.GetUserMarkById(markId); ASSERT(userMark != nullptr, ()); ASSERT(dynamic_cast(userMark) != nullptr, ()); diff --git a/map/bookmark_manager.hpp b/map/bookmark_manager.hpp index f97c01d252..68f7f1199a 100644 --- a/map/bookmark_manager.hpp +++ b/map/bookmark_manager.hpp @@ -26,15 +26,10 @@ class BookmarkManager final using CategoryIter = CategoriesCollection::iterator; using UserMarkLayers = std::vector>; - using GetStringsBundleFn = std::function; - public: using AsyncLoadingStartedCallback = std::function; using AsyncLoadingFinishedCallback = std::function; using AsyncLoadingFileCallback = std::function; - using CreatedBookmarksCallback = std::function> const &)>; - using UpdatedBookmarksCallback = std::function> const &)>; - using DeletedBookmarksCallback = std::function const &)>; struct AsyncLoadingCallbacks { @@ -44,10 +39,29 @@ public: AsyncLoadingFileCallback m_onFileSuccess; }; - explicit BookmarkManager(GetStringsBundleFn && getStringsBundleFn, - CreatedBookmarksCallback && createdBookmarksCallback, - UpdatedBookmarksCallback && updatedBookmarksCallback, - DeletedBookmarksCallback && deletedBookmarksCallback); + struct Callbacks + { + using GetStringsBundleFn = std::function; + using CreatedBookmarksCallback = std::function> const &)>; + using UpdatedBookmarksCallback = std::function> const &)>; + using DeletedBookmarksCallback = std::function const &)>; + + template + Callbacks(StringsBundleGetter && stringsBundleGetter, CreateListener && createListener, + UpdateListener && updateListener, DeleteListener && deleteListener) + : m_getStringsBundle(std::forward(stringsBundleGetter)) + , m_createdBookmarksCallback(std::forward(createListener)) + , m_updatedBookmarksCallback(std::forward(updateListener)) + , m_deletedBookmarksCallback(std::forward(deleteListener)) + {} + + GetStringsBundleFn m_getStringsBundle; + CreatedBookmarksCallback m_createdBookmarksCallback; + UpdatedBookmarksCallback m_updatedBookmarksCallback; + DeletedBookmarksCallback m_deletedBookmarksCallback; + }; + + explicit BookmarkManager(Callbacks && callbacks); ~BookmarkManager(); void SetDrapeEngine(ref_ptr engine); @@ -113,16 +127,13 @@ private: void NotifyAboutFile(bool success, std::string const & filePath, bool isTemporaryFile); void LoadBookmarkRoutine(std::string const & filePath, bool isTemporaryFile); - void OnCreateUserMarks(UserMarkContainer * container, df::IDCollection const & markIds); - void OnUpdateUserMarks(UserMarkContainer * container, df::IDCollection const & markIds); - void OnDeleteUserMarks(UserMarkContainer * container, df::IDCollection const & markIds); - void GetBookmarksData(UserMarkContainer * container, df::IDCollection const & markIds, + void OnCreateUserMarks(UserMarkContainer const & container, df::IDCollection const & markIds); + void OnUpdateUserMarks(UserMarkContainer const & container, df::IDCollection const & markIds); + void OnDeleteUserMarks(UserMarkContainer const & container, df::IDCollection const & markIds); + void GetBookmarksData(UserMarkContainer const & container, df::IDCollection const & markIds, std::vector> & data) const; - GetStringsBundleFn m_getStringsBundle; - CreatedBookmarksCallback m_createdBookmarksCallback; - UpdatedBookmarksCallback m_updatedBookmarksCallback; - DeletedBookmarksCallback m_deletedBookmarksCallback; + Callbacks m_callbacks; UserMarkContainer::Listeners m_bookmarksListeners; df::DrapeEngineSafePtr m_drapeEngine; diff --git a/map/framework.cpp b/map/framework.cpp index 9825b6d736..5bd78a1d30 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -359,19 +359,19 @@ Framework::Framework(FrameworkParams const & params) : m_startForegroundTime(0.0) , m_storage(platform::migrate::NeedMigrate() ? COUNTRIES_OBSOLETE_FILE : COUNTRIES_FILE) , m_enabledDiffs(params.m_enableDiffs) - , m_bmManager([this]() -> StringsBundle const & { return m_stringsBundle; }, - [](std::vector> const & marks) - { - // TODO: Add processing of the created marks. - }, - [](std::vector> const & marks) - { - // TODO: Add processing of the updated marks. - }, - [](std::vector const & marks) - { - // TODO: Add processing of the deleted marks. - }) + , m_bmManager(BookmarkManager::Callbacks([this]() -> StringsBundle const & { return m_stringsBundle; }, + [](std::vector> const & marks) + { + // TODO: Add processing of the created marks. + }, + [](std::vector> const & marks) + { + // TODO: Add processing of the updated marks. + }, + [](std::vector const & marks) + { + // TODO: Add processing of the deleted marks. + })) , m_isRenderingEnabled(true) , m_routingManager(RoutingManager::Callbacks([this]() -> Index & { return m_model.GetIndex(); }, [this]() -> storage::CountryInfoGetter & { return GetCountryInfoGetter(); }, diff --git a/map/map_tests/bookmarks_test.cpp b/map/map_tests/bookmarks_test.cpp index 3a214e1bbf..d201259f5d 100644 --- a/map/map_tests/bookmarks_test.cpp +++ b/map/map_tests/bookmarks_test.cpp @@ -13,8 +13,11 @@ #include "coding/internal/file_data.hpp" -#include "std/fstream.hpp" -#include "std/unique_ptr.hpp" +#include +#include +#include + +using namespace std; namespace { @@ -652,3 +655,83 @@ UNIT_TEST(TrackParsingTest_2) TEST_GREATER(track->GetLayerCount(), 0, ()); TEST_EQUAL(track->GetColor(0), dp::Color(57, 255, 32, 255), ()); } + +UNIT_TEST(Bookmarks_Listeners) +{ + set createdMarksResult; + set updatedMarksResult; + set deletedMarksResult; + set createdMarks; + set updatedMarks; + set deletedMarks; + + auto const checkNotifications = [&](BookmarkCategory & cat) + { + cat.NotifyChanges(); + TEST_EQUAL(createdMarks, createdMarksResult, ()); + TEST_EQUAL(updatedMarks, updatedMarksResult, ()); + TEST_EQUAL(deletedMarks, deletedMarksResult, ()); + }; + + auto const refresh = [&](BookmarkCategory & cat) + { + df::MarkIDCollection createdIds; + df::MarkIDCollection deletedIds; + cat.AcceptChanges(createdIds, deletedIds); + + createdMarksResult.clear(); + updatedMarksResult.clear(); + deletedMarksResult.clear(); + createdMarks.clear(); + updatedMarks.clear(); + deletedMarks.clear(); + }; + + auto const onCreate = [&createdMarksResult](UserMarkContainer const & container, df::IDCollection const & markIds) + { + createdMarksResult.insert(markIds.begin(), markIds.end()); + }; + auto const onUpdate = [&updatedMarksResult](UserMarkContainer const & container, df::IDCollection const & markIds) + { + updatedMarksResult.insert(markIds.begin(), markIds.end()); + }; + auto const onDelete = [&deletedMarksResult](UserMarkContainer const & container, df::IDCollection const & markIds) + { + deletedMarksResult.insert(markIds.begin(), markIds.end()); + }; + + BookmarkCategory cat("Default", UserMarkContainer::Listeners(onCreate, onUpdate, onDelete)); + + auto bookmark0 = static_cast(cat.CreateUserMark(m2::PointD(0.0, 0.0))); + bookmark0->SetData(BookmarkData("name 0", "type 0")); + createdMarks.insert(bookmark0->GetId()); + + auto bookmark1 = static_cast(cat.CreateUserMark(m2::PointD(0.0, 0.0))); + bookmark1->SetData(BookmarkData("name 1", "type 1")); + createdMarks.insert(bookmark1->GetId()); + + createdMarks.erase(cat.GetUserMark(0)->GetId()); + cat.DeleteUserMark(0); + + checkNotifications(cat); + refresh(cat); + + bookmark0 = static_cast(cat.GetUserMarkForEdit(0)); + bookmark0->SetName("name 3"); + updatedMarks.insert(cat.GetUserMark(0)->GetId()); + + checkNotifications(cat); + refresh(cat); + + deletedMarks.insert(cat.GetUserMark(0)->GetId()); + bookmark0 = static_cast(cat.GetUserMarkForEdit(0)); + bookmark0->SetName("name 4"); + cat.DeleteUserMark(0); + + bookmark1 = static_cast(cat.CreateUserMark(m2::PointD(0.0, 0.0))); + createdMarks.insert(bookmark1->GetId()); + bookmark1->SetData(BookmarkData("name 5", "type 5")); + + checkNotifications(cat); + refresh(cat); +} diff --git a/map/user_mark_container.cpp b/map/user_mark_container.cpp index 0e7174450c..cc36c185d4 100644 --- a/map/user_mark_container.cpp +++ b/map/user_mark_container.cpp @@ -73,11 +73,6 @@ void UserMarkContainer::SetDrapeEngine(ref_ptr engine) m_drapeEngine.Set(engine); } -void UserMarkContainer::SetListeners(Listeners const & listeners) -{ - m_listeners = listeners; -} - UserMark const * UserMarkContainer::GetUserMarkById(df::MarkID id) const { for (auto const & mark : m_userMarks) @@ -111,7 +106,7 @@ void UserMarkContainer::NotifyListeners() if (m_listeners.m_createListener != nullptr && !m_createdMarks.empty()) { df::IDCollection marks(m_createdMarks.begin(), m_createdMarks.end()); - m_listeners.m_createListener(this, marks); + m_listeners.m_createListener(*this, marks); } if (m_listeners.m_updateListener != nullptr) { @@ -122,12 +117,12 @@ void UserMarkContainer::NotifyListeners() marks.push_back(mark->GetId()); } if (!marks.empty()) - m_listeners.m_updateListener(this, marks); + m_listeners.m_updateListener(*this, marks); } if (m_listeners.m_deleteListener != nullptr && !m_removedMarks.empty()) { df::IDCollection marks(m_removedMarks.begin(), m_removedMarks.end()); - m_listeners.m_deleteListener(this, marks); + m_listeners.m_deleteListener(*this, marks); } } diff --git a/map/user_mark_container.hpp b/map/user_mark_container.hpp index 04ca6c49e6..f875cc5918 100644 --- a/map/user_mark_container.hpp +++ b/map/user_mark_container.hpp @@ -39,7 +39,7 @@ class UserMarkContainer : public df::UserMarksProvider { public: using TUserMarksList = std::deque>; - using NotifyChangesFn = std::function; + using NotifyChangesFn = std::function; struct Listeners { @@ -62,7 +62,6 @@ public: ~UserMarkContainer() override; void SetDrapeEngine(ref_ptr engine); - void SetListeners(Listeners const & listeners); UserMark const * GetUserMarkById(df::MarkID id) const; // If not found mark on rect result is nullptr.