From 18c863bb3c8098ee96f67c907e66d3bb805c404b Mon Sep 17 00:00:00 2001
From: Arsentiy Milchakov <milcars@mapswithme.com>
Date: Tue, 10 Dec 2019 14:19:41 +0300
Subject: [PATCH] [notification] review fixes

---
 map/map_tests/notification_tests.cpp                | 10 ++++++++--
 map/notifications/notification_manager.cpp          | 13 +++++--------
 map/notifications/notification_manager.hpp          |  4 ++--
 map/notifications/notification_manager_delegate.cpp |  4 ++--
 map/notifications/notification_manager_delegate.hpp |  4 ++--
 5 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/map/map_tests/notification_tests.cpp b/map/map_tests/notification_tests.cpp
index b53d8c7d1c..b51601a877 100644
--- a/map/map_tests/notification_tests.cpp
+++ b/map/map_tests/notification_tests.cpp
@@ -34,12 +34,12 @@ public:
     }
 
     std::unordered_set<storage::CountryId> GetDescendantCountries(
-        storage::CountryId const & country) override
+        storage::CountryId const & country) const override
     {
       return {{"South Korea_North"}, {"South Korea_South"}};
     }
 
-    storage::CountryId GetCountryAtPoint(m2::PointD const & pt) override
+    storage::CountryId GetCountryAtPoint(m2::PointD const & pt) const override
     {
       return "South Korea_North";
     }
@@ -328,5 +328,11 @@ UNIT_CLASS_TEST(ScopedNotificationsQueue, Notifications_DeleteCanidatesForCountr
   TEST_EQUAL(notificationManager.GetEditableQueue().m_candidates.size(), 1, ());
   notificationManager.DeleteCandidatesForCountry("South Korea_North");
   TEST_EQUAL(notificationManager.GetEditableQueue().m_candidates.size(), 0, ());
+
+  notificationManager.GetEditableQueue().m_candidates.push_back({mapObject, ""});
+
+  TEST_EQUAL(notificationManager.GetEditableQueue().m_candidates.size(), 1, ());
+  notificationManager.DeleteCandidatesForCountry("South Korea");
+  TEST_EQUAL(notificationManager.GetEditableQueue().m_candidates.size(), 0, ());
 }
 }  // namespace
diff --git a/map/notifications/notification_manager.cpp b/map/notifications/notification_manager.cpp
index 98cb4dbd38..30cc286c0a 100644
--- a/map/notifications/notification_manager.cpp
+++ b/map/notifications/notification_manager.cpp
@@ -185,14 +185,11 @@ void NotificationManager::DeleteCandidatesForCountry(storage::CountryId const &
   auto & candidates = m_queue.m_candidates;
   size_t const sizeBefore = candidates.size();
 
-  candidates.erase(std::remove_if(candidates.begin(), candidates.end(),
-                                  [this, &countries](auto const & item) {
-                                    auto const itemCountry =
-                                        m_delegate->GetCountryAtPoint(item.GetPos());
-                                    return std::find(countries.cbegin(), countries.cend(),
-                                                     itemCountry) != countries.cend();
-                                  }),
-                   candidates.end());
+  base::EraseIf(candidates, [this, &countries](auto const & item)
+  {
+    auto const itemCountry = m_delegate->GetCountryAtPoint(item.GetPos());
+    return countries.count(itemCountry) != 0;
+  });
 
   if (sizeBefore != candidates.size())
     VERIFY(Save(), ());
diff --git a/map/notifications/notification_manager.hpp b/map/notifications/notification_manager.hpp
index 3470547a44..e20108a760 100644
--- a/map/notifications/notification_manager.hpp
+++ b/map/notifications/notification_manager.hpp
@@ -33,8 +33,8 @@ public:
     virtual ~Delegate() = default;
     virtual ugc::Api & GetUGCApi() = 0;
     virtual std::unordered_set<storage::CountryId> GetDescendantCountries(
-        storage::CountryId const & country) = 0;
-    virtual storage::CountryId GetCountryAtPoint(m2::PointD const & pt) = 0;
+        storage::CountryId const & country) const = 0;
+    virtual storage::CountryId GetCountryAtPoint(m2::PointD const & pt) const = 0;
     virtual std::string GetAddress(m2::PointD const & pt) = 0;
   };
 
diff --git a/map/notifications/notification_manager_delegate.cpp b/map/notifications/notification_manager_delegate.cpp
index 6e93de0ab9..c48c6c7cac 100644
--- a/map/notifications/notification_manager_delegate.cpp
+++ b/map/notifications/notification_manager_delegate.cpp
@@ -34,7 +34,7 @@ ugc::Api & NotificationManagerDelegate::GetUGCApi()
 }
 
 std::unordered_set<storage::CountryId> NotificationManagerDelegate::GetDescendantCountries(
-    storage::CountryId const & country)
+    storage::CountryId const & country) const
 {
   std::unordered_set<storage::CountryId> result;
   auto const fn = [&result](storage::CountryId const & countryId, bool isGroupNode)
@@ -48,7 +48,7 @@ std::unordered_set<storage::CountryId> NotificationManagerDelegate::GetDescendan
   return result;
 }
 
-storage::CountryId NotificationManagerDelegate::GetCountryAtPoint(m2::PointD const & pt)
+storage::CountryId NotificationManagerDelegate::GetCountryAtPoint(m2::PointD const & pt) const
 {
   return m_countryInfoGetter.GetRegionCountryId(pt);
 }
diff --git a/map/notifications/notification_manager_delegate.hpp b/map/notifications/notification_manager_delegate.hpp
index 697d811d71..1aa9738f7e 100644
--- a/map/notifications/notification_manager_delegate.hpp
+++ b/map/notifications/notification_manager_delegate.hpp
@@ -40,8 +40,8 @@ public:
   // NotificationManager::Delegate overrides:
   ugc::Api & GetUGCApi() override;
   std::unordered_set<storage::CountryId> GetDescendantCountries(
-      storage::CountryId const & country) override;
-  storage::CountryId GetCountryAtPoint(m2::PointD const & pt) override;
+      storage::CountryId const & country) const override;
+  storage::CountryId GetCountryAtPoint(m2::PointD const & pt) const override;
   std::string GetAddress(m2::PointD const & pt) override;
 
 private: