[storage] A refactoring of CountryInfoGetter.

10 files changed, 216 insertions, 203 deletions

diff --git a/storage/country_decl.cpp b/storage/country_decl.cpp
index 1abb3c2a0c..72edc18be7 100644
--- a/storage/country_decl.cpp
+++ b/storage/country_decl.cpp
@@ -1,7 +1,13 @@
 #include "storage/country_decl.hpp"
 
+#include <cstddef>
 
-void storage::CountryInfo::FileName2FullName(string & fName)
+using namespace std;
+
+namespace storage
+{
+// static
+void CountryInfo::FileName2FullName(string & fName)
 {
   size_t const i = fName.find('_');
   if (i != string::npos)
@@ -12,7 +18,8 @@ void storage::CountryInfo::FileName2FullName(string & fName)
   }
 }
 
-void storage::CountryInfo::FullName2GroupAndMap(string const & fName, string & group, string & map)
+// static
+void CountryInfo::FullName2GroupAndMap(string const & fName, string & group, string & map)
 {
   size_t pos = fName.find(",");
   if (pos == string::npos)
@@ -26,3 +33,4 @@ void storage::CountryInfo::FullName2GroupAndMap(string const & fName, string & g
     group = fName.substr(0, pos);
   }
 }
+}  // namespace storage
diff --git a/storage/country_decl.hpp b/storage/country_decl.hpp
index d83066e2be..0b608215a3 100644
--- a/storage/country_decl.hpp
+++ b/storage/country_decl.hpp
@@ -8,34 +8,34 @@
 
 namespace storage
 {
-  struct CountryDef
-  {
-    /// File name without extension (equal to english name - used in search for region).
-    CountryId m_countryId;
-    m2::RectD m_rect;
-
-    CountryDef() {}
-    CountryDef(CountryId const & countryId, m2::RectD const & rect)
-      : m_countryId(countryId), m_rect(rect)
-    {
-    }
-  };
-
-  struct CountryInfo
+/// File name without extension (equal to english name - used in search for region).
+struct CountryDef
+{
+  CountryDef() = default;
+  CountryDef(CountryId const & countryId, m2::RectD const & rect)
+    : m_countryId(countryId), m_rect(rect)
   {
-    CountryInfo() = default;
-    CountryInfo(std::string const & id) : m_name(id) {}
-
-    /// Name (in native language) of country or region.
-    /// (if empty - equals to file name of country - no additional memory)
-    std::string m_name;
-
-    bool IsNotEmpty() const { return !m_name.empty(); }
-
-    // @TODO(bykoianko) Twine will be used intead of this function.
-    // So id (fName) will be converted to a local name.
-    static void FileName2FullName(std::string & fName);
-    static void FullName2GroupAndMap(std::string const & fName, std::string & group,
-                                     std::string & map);
-  };
-}
+  }
+
+  CountryId m_countryId;
+  m2::RectD m_rect;
+};
+
+struct CountryInfo
+{
+  CountryInfo() = default;
+  CountryInfo(std::string const & id) : m_name(id) {}
+
+  // @TODO(bykoianko) Twine will be used intead of this function.
+  // So id (fName) will be converted to a local name.
+  static void FileName2FullName(std::string & fName);
+  static void FullName2GroupAndMap(std::string const & fName, std::string & group,
+                                   std::string & map);
+
+  bool IsNotEmpty() const { return !m_name.empty(); }
+
+  /// Name (in native language) of country or region.
+  /// (if empty - equals to file name of country - no additional memory)
+  std::string m_name;
+};
+}  // namespace storage
diff --git a/storage/country_info_getter.cpp b/storage/country_info_getter.cpp
index f81541f076..0d6d9515ba 100644
--- a/storage/country_info_getter.cpp
+++ b/storage/country_info_getter.cpp
@@ -5,6 +5,7 @@
 
 #include "platform/local_country_file_utils.hpp"
 
+#include "coding/geometry_coding.hpp"
 #include "coding/read_write_utils.hpp"
 
 #include "geometry/latlon.hpp"
@@ -14,62 +15,39 @@
 #include "base/logging.hpp"
 #include "base/string_utils.hpp"
 
-#include "3party/Alohalytics/src/alohalytics.h"
-
 #include <functional>
 #include <limits>
+#include <utility>
+
+#include "3party/Alohalytics/src/alohalytics.h"
 
 namespace storage
 {
 namespace
 {
 size_t const kInvalidId = std::numeric_limits<size_t>::max();
-
-struct DoFreeCacheMemory
-{
-  void operator()(std::vector<m2::RegionD> & v) const { std::vector<m2::RegionD>().swap(v); }
-};
-
-class DoCalcUSA
-{
-public:
-  explicit DoCalcUSA(m2::RectD * rects) : m_rects(rects) {}
-
-  void operator()(CountryDef const & c)
-  {
-    if (c.m_countryId == "USA_Alaska")
-      m_rects[1] = c.m_rect;
-    else if (c.m_countryId == "USA_Hawaii")
-      m_rects[2] = c.m_rect;
-    else
-      m_rects[0].Add(c.m_rect);
-  }
-
-private:
-  m2::RectD * const m_rects;
-};
 }  // namespace
 
 // CountryInfoGetterBase ---------------------------------------------------------------------------
 CountryId CountryInfoGetterBase::GetRegionCountryId(m2::PointD const & pt) const
 {
-  TRegionId const id = FindFirstCountry(pt);
-  return id != kInvalidId ? m_countries[id].m_countryId : kInvalidCountryId;
+  RegionId const id = FindFirstCountry(pt);
+  return id == kInvalidId ? kInvalidCountryId : m_countries[id].m_countryId;
 }
 
-bool CountryInfoGetterBase::IsBelongToRegions(m2::PointD const & pt,
-                                              TRegionIdSet const & regions) const
+bool CountryInfoGetterBase::BelongsToAnyRegion(m2::PointD const & pt,
+                                               RegionIdVec const & regions) const
 {
   for (auto const & id : regions)
   {
-    if (m_countries[id].m_rect.IsPointInside(pt) && IsBelongToRegionImpl(id, pt))
+    if (BelongsToRegion(pt, id))
       return true;
   }
   return false;
 }
 
-bool CountryInfoGetterBase::IsBelongToRegions(CountryId const & countryId,
-                                              TRegionIdSet const & regions) const
+bool CountryInfoGetterBase::BelongsToAnyRegion(CountryId const & countryId,
+                                               RegionIdVec const & regions) const
 {
   for (auto const & id : regions)
   {
@@ -79,18 +57,11 @@ bool CountryInfoGetterBase::IsBelongToRegions(CountryId const & countryId,
   return false;
 }
 
-void CountryInfoGetterBase::RegionIdsToCountryIds(TRegionIdSet const & regions,
-                                                  CountriesVec & countries) const
-{
-  for (auto const & id : regions)
-    countries.push_back(m_countries[id].m_countryId);
-}
-
-CountryInfoGetterBase::TRegionId CountryInfoGetterBase::FindFirstCountry(m2::PointD const & pt) const
+CountryInfoGetterBase::RegionId CountryInfoGetterBase::FindFirstCountry(m2::PointD const & pt) const
 {
   for (size_t id = 0; id < m_countries.size(); ++id)
   {
-    if (m_countries[id].m_rect.IsPointInside(pt) && IsBelongToRegionImpl(id, pt))
+    if (BelongsToRegion(pt, id))
       return id;
   }
 
@@ -101,10 +72,7 @@ CountryInfoGetterBase::TRegionId CountryInfoGetterBase::FindFirstCountry(m2::Poi
 vector<CountryId> CountryInfoGetter::GetRegionsCountryIdByRect(m2::RectD const & rect,
                                                                bool rough) const
 {
-  size_t constexpr kAverageSize = 10;
-
   std::vector<CountryId> result;
-  result.reserve(kAverageSize);
   for (size_t id = 0; id < m_countries.size(); ++id)
   {
     if (rect.IsRectInside(m_countries[id].m_rect))
@@ -113,7 +81,7 @@ vector<CountryId> CountryInfoGetter::GetRegionsCountryIdByRect(m2::RectD const &
     }
     else if (rect.IsIntersect(m_countries[id].m_rect))
     {
-      if (rough || IsIntersectedByRegionImpl(id, rect))
+      if (rough || IsIntersectedByRegion(rect, id))
         result.push_back(m_countries[id].m_countryId);
     }
   }
@@ -137,15 +105,15 @@ void CountryInfoGetter::GetRegionsCountryId(m2::PointD const & pt, CountriesVec
 
 void CountryInfoGetter::GetRegionInfo(m2::PointD const & pt, CountryInfo & info) const
 {
-  TRegionId const id = FindFirstCountry(pt);
+  RegionId const id = FindFirstCountry(pt);
   if (id != kInvalidId)
     GetRegionInfo(m_countries[id].m_countryId, info);
 }
 
 void CountryInfoGetter::GetRegionInfo(CountryId const & countryId, CountryInfo & info) const
 {
-  auto const it = m_id2info.find(countryId);
-  if (it == m_id2info.end())
+  auto const it = m_idToInfo.find(countryId);
+  if (it == m_idToInfo.end())
     return;
 
   info = it->second;
@@ -157,10 +125,19 @@ void CountryInfoGetter::GetRegionInfo(CountryId const & countryId, CountryInfo &
 
 void CountryInfoGetter::CalcUSALimitRect(m2::RectD rects[3]) const
 {
-  ForEachCountry("USA_", DoCalcUSA(rects));
+  auto fn = [&](CountryDef const & c) {
+    if (c.m_countryId == "USA_Alaska")
+      rects[1] = c.m_rect;
+    else if (c.m_countryId == "USA_Hawaii")
+      rects[2] = c.m_rect;
+    else
+      rects[0].Add(c.m_rect);
+  };
+
+  ForEachCountry("USA_", fn);
 }
 
-m2::RectD CountryInfoGetter::CalcLimitRect(string const & prefix) const
+m2::RectD CountryInfoGetter::CalcLimitRect(std::string const & prefix) const
 {
   m2::RectD rect;
   ForEachCountry(prefix, [&rect](CountryDef const & c)
@@ -178,7 +155,8 @@ m2::RectD CountryInfoGetter::GetLimitRectForLeaf(CountryId const & leafCountryId
   return m_countries[it->second].m_rect;
 }
 
-void CountryInfoGetter::GetMatchedRegions(string const & affiliation, TRegionIdSet & regions) const
+void CountryInfoGetter::GetMatchedRegions(std::string const & affiliation,
+                                          RegionIdVec & regions) const
 {
   CHECK(m_affiliations, ());
   auto it = m_affiliations->find(affiliation);
@@ -197,9 +175,8 @@ void CountryInfoGetter::SetAffiliations(Affiliations const * affiliations)
   m_affiliations = affiliations;
 }
 
-
 template <typename ToDo>
-void CountryInfoGetter::ForEachCountry(string const & prefix, ToDo && toDo) const
+void CountryInfoGetter::ForEachCountry(std::string const & prefix, ToDo && toDo) const
 {
   for (auto const & country : m_countries)
   {
@@ -210,91 +187,97 @@ void CountryInfoGetter::ForEachCountry(string const & prefix, ToDo && toDo) cons
 
 // CountryInfoReader -------------------------------------------------------------------------------
 // static
-unique_ptr<CountryInfoGetter> CountryInfoReader::CreateCountryInfoReader(Platform const & platform)
+std::unique_ptr<CountryInfoGetter> CountryInfoReader::CreateCountryInfoReader(
+    Platform const & platform)
 {
   try
   {
     CountryInfoReader * result = new CountryInfoReader(platform.GetReader(PACKED_POLYGONS_FILE),
                                                        platform.GetReader(COUNTRIES_FILE));
-    return unique_ptr<CountryInfoReader>(result);
+    return std::unique_ptr<CountryInfoReader>(result);
   }
   catch (RootException const & e)
   {
     LOG(LCRITICAL, ("Can't load needed resources for storage::CountryInfoGetter:", e.Msg()));
   }
-  return unique_ptr<CountryInfoReader>();
+  return std::unique_ptr<CountryInfoReader>();
 }
 
 // static
-unique_ptr<CountryInfoGetter> CountryInfoReader::CreateCountryInfoReaderObsolete(
+std::unique_ptr<CountryInfoGetter> CountryInfoReader::CreateCountryInfoReaderObsolete(
     Platform const & platform)
 {
   try
   {
     CountryInfoReader * result = new CountryInfoReader(platform.GetReader(PACKED_POLYGONS_OBSOLETE_FILE),
                                                        platform.GetReader(COUNTRIES_OBSOLETE_FILE));
-    return unique_ptr<CountryInfoReader>(result);
+    return std::unique_ptr<CountryInfoReader>(result);
   }
   catch (RootException const & e)
   {
     LOG(LCRITICAL, ("Can't load needed resources for storage::CountryInfoGetter:", e.Msg()));
   }
-  return unique_ptr<CountryInfoReader>();
+  return std::unique_ptr<CountryInfoReader>();
 }
 
 CountryInfoReader::CountryInfoReader(ModelReaderPtr polyR, ModelReaderPtr countryR)
-  : CountryInfoGetter(true), m_reader(polyR), m_cache(3)
+  : CountryInfoGetter(true), m_reader(polyR), m_cache(3 /* logCacheSize */)
+
 {
   ReaderSource<ModelReaderPtr> src(m_reader.GetReader(PACKED_POLYGONS_INFO_TAG));
   rw::Read(src, m_countries);
 
-  size_t const countrySz = m_countries.size();
-  m_countryIndex.reserve(countrySz);
-  for (size_t i = 0; i < countrySz; ++i)
+  m_countryIndex.reserve(m_countries.size());
+  for (size_t i = 0; i < m_countries.size(); ++i)
     m_countryIndex[m_countries[i].m_countryId] = i;
 
-  string buffer;
+  std::string buffer;
   countryR.ReadAsString(buffer);
-  LoadCountryFile2CountryInfo(buffer, m_id2info, m_isSingleMwm);
+  LoadCountryFile2CountryInfo(buffer, m_idToInfo, m_isSingleMwm);
 }
 
 void CountryInfoReader::ClearCachesImpl() const
 {
   std::lock_guard<std::mutex> lock(m_cacheMutex);
 
-  m_cache.ForEachValue(DoFreeCacheMemory());
+  m_cache.ForEachValue([](std::vector<m2::RegionD> & v) { std::vector<m2::RegionD>().swap(v); });
   m_cache.Reset();
 }
 
-template <typename TFn>
-std::result_of_t<TFn(vector<m2::RegionD>)> CountryInfoReader::WithRegion(size_t id, TFn && fn) const
+template <typename Fn>
+std::result_of_t<Fn(std::vector<m2::RegionD>)> CountryInfoReader::WithRegion(size_t id,
+                                                                             Fn && fn) const
 {
   std::lock_guard<std::mutex> lock(m_cacheMutex);
 
   bool isFound = false;
-  std::vector<m2::RegionD> & rgns = m_cache.Find(static_cast<uint32_t>(id), isFound);
+  auto & regions = m_cache.Find(static_cast<uint32_t>(id), isFound);
 
   if (!isFound)
-  {
-    rgns.clear();
-    // Load regions from file.
-    ReaderSource<ModelReaderPtr> src(m_reader.GetReader(strings::to_string(id)));
-
-    uint32_t const count = ReadVarUint<uint32_t>(src);
-    for (size_t i = 0; i < count; ++i)
-    {
-      std::vector<m2::PointD> points;
-      serial::LoadOuterPath(src, serial::GeometryCodingParams(), points);
-      rgns.emplace_back(move(points));
-    }
-  }
+    LoadRegionsFromDisk(id, regions);
 
-  return fn(rgns);
+  return fn(regions);
 }
 
+void CountryInfoReader::LoadRegionsFromDisk(size_t id, std::vector<m2::RegionD> & regions) const
+{
+  regions.clear();
+  ReaderSource<ModelReaderPtr> src(m_reader.GetReader(strings::to_string(id)));
+
+  uint32_t const count = ReadVarUint<uint32_t>(src);
+  for (size_t i = 0; i < count; ++i)
+  {
+    std::vector<m2::PointD> points;
+    serial::LoadOuterPath(src, serial::GeometryCodingParams(), points);
+    regions.emplace_back(std::move(points));
+  }
+}
 
-bool CountryInfoReader::IsBelongToRegionImpl(size_t id, m2::PointD const & pt) const
+bool CountryInfoReader::BelongsToRegion(m2::PointD const & pt, size_t id) const
 {
+  if (!m_countries[id].m_rect.IsPointInside(pt))
+    return false;
+
   auto contains = [&pt](std::vector<m2::RegionD> const & regions) {
     for (auto const & region : regions)
     {
@@ -307,12 +290,13 @@ bool CountryInfoReader::IsBelongToRegionImpl(size_t id, m2::PointD const & pt) c
   return WithRegion(id, contains);
 }
 
-bool CountryInfoReader::IsIntersectedByRegionImpl(size_t id, m2::RectD const & rect) const
+bool CountryInfoReader::IsIntersectedByRegion(m2::RectD const & rect, size_t id) const
 {
-  std::vector<pair<m2::PointD, m2::PointD>> edges = {{rect.LeftTop(), rect.RightTop()},
-                                                     {rect.RightTop(), rect.RightBottom()},
-                                                     {rect.RightBottom(), rect.LeftBottom()},
-                                                     {rect.LeftBottom(), rect.LeftTop()}};
+  std::vector<std::pair<m2::PointD, m2::PointD>> const edges = {
+      {rect.LeftTop(), rect.RightTop()},
+      {rect.RightTop(), rect.RightBottom()},
+      {rect.RightBottom(), rect.LeftBottom()},
+      {rect.LeftBottom(), rect.LeftTop()}};
   auto contains = [&edges](std::vector<m2::RegionD> const & regions) {
     for (auto const & region : regions)
     {
@@ -329,7 +313,7 @@ bool CountryInfoReader::IsIntersectedByRegionImpl(size_t id, m2::RectD const & r
   if (WithRegion(id, contains))
     return true;
 
-  return IsBelongToRegionImpl(id, rect.Center());
+  return BelongsToRegion(rect.Center(), id);
 }
 
 bool CountryInfoReader::IsCloseEnough(size_t id, m2::PointD const & pt, double distance)
@@ -358,12 +342,12 @@ CountryInfoGetterForTesting::CountryInfoGetterForTesting(std::vector<CountryDef>
 void CountryInfoGetterForTesting::AddCountry(CountryDef const & country)
 {
   m_countries.push_back(country);
-  string const & name = country.m_countryId;
-  m_id2info[name].m_name = name;
+  std::string const & name = country.m_countryId;
+  m_idToInfo[name].m_name = name;
 }
 
-void CountryInfoGetterForTesting::GetMatchedRegions(string const & affiliation,
-                                                    TRegionIdSet & regions) const
+void CountryInfoGetterForTesting::GetMatchedRegions(std::string const & affiliation,
+                                                    RegionIdVec & regions) const
 {
   for (size_t i = 0; i < m_countries.size(); ++i)
   {
@@ -374,14 +358,13 @@ void CountryInfoGetterForTesting::GetMatchedRegions(string const & affiliation,
 
 void CountryInfoGetterForTesting::ClearCachesImpl() const {}
 
-bool CountryInfoGetterForTesting::IsBelongToRegionImpl(size_t id,
-                                                       m2::PointD const & pt) const
+bool CountryInfoGetterForTesting::BelongsToRegion(m2::PointD const & pt, size_t id) const
 {
   CHECK_LESS(id, m_countries.size(), ());
   return m_countries[id].m_rect.IsPointInside(pt);
 }
 
-bool CountryInfoGetterForTesting::IsIntersectedByRegionImpl(size_t id, m2::RectD const & rect) const
+bool CountryInfoGetterForTesting::IsIntersectedByRegion(m2::RectD const & rect, size_t id) const
 {
   CHECK_LESS(id, m_countries.size(), ());
   return rect.IsIntersect(m_countries[id].m_rect);
diff --git a/storage/country_info_getter.hpp b/storage/country_info_getter.hpp
index c37c2e9eab..0084f6c170 100644
--- a/storage/country_info_getter.hpp
+++ b/storage/country_info_getter.hpp
@@ -6,12 +6,15 @@
 
 #include "platform/platform.hpp"
 
+#include "geometry/point2d.hpp"
 #include "geometry/region2d.hpp"
 
 #include "coding/file_container.hpp"
 
 #include "base/cache.hpp"
 
+#include <cstddef>
+#include <cstdint>
 #include <map>
 #include <memory>
 #include <mutex>
@@ -27,8 +30,8 @@ class CountryInfoGetterBase
 {
 public:
   // Identifier of a region (index in m_countries array).
-  using TRegionId = size_t;
-  using TRegionIdSet = std::vector<TRegionId>;
+  using RegionId = size_t;
+  using RegionIdVec = std::vector<RegionId>;
 
   explicit CountryInfoGetterBase(bool isSingleMwm) : m_isSingleMwm(isSingleMwm) {}
   virtual ~CountryInfoGetterBase() = default;
@@ -40,22 +43,21 @@ public:
 
   // Returns true when |pt| belongs to at least one of the specified
   // |regions|.
-  bool IsBelongToRegions(m2::PointD const & pt, TRegionIdSet const & regions) const;
+  bool BelongsToAnyRegion(m2::PointD const & pt, RegionIdVec const & regions) const;
 
-  // Returns true if there're at least one region with id equal to |countryId|.
-  bool IsBelongToRegions(CountryId const & countryId, TRegionIdSet const & regions) const;
-
-  void RegionIdsToCountryIds(TRegionIdSet const & regions, CountriesVec & countries) const;
+  // Returns true if there's at least one region with id equal to |countryId|.
+  bool BelongsToAnyRegion(CountryId const & countryId, RegionIdVec const & regions) const;
 
 protected:
-  // Returns identifier of the first country containing |pt|.
-  TRegionId FindFirstCountry(m2::PointD const & pt) const;
+  // Returns identifier of the first country containing |pt| or |kInvalidId| if there is none.
+  RegionId FindFirstCountry(m2::PointD const & pt) const;
 
-  // Returns true when |pt| belongs to a country identified by |id|.
-  virtual bool IsBelongToRegionImpl(size_t id, m2::PointD const & pt) const = 0;
+  // Returns true when |pt| belongs to the country identified by |id|.
+  virtual bool BelongsToRegion(m2::PointD const & pt, size_t id) const = 0;
 
   // List of all known countries.
   std::vector<CountryDef> m_countries;
+
   // m_isSingleMwm == true if the system is currently working with single (small) mwms
   // and false otherwise.
   // @TODO(bykoianko) Init m_isSingleMwm correctly.
@@ -68,68 +70,69 @@ class CountryInfoGetter : public CountryInfoGetterBase
 public:
   explicit CountryInfoGetter(bool isSingleMwm) : CountryInfoGetterBase(isSingleMwm) {}
 
-  // Returns vector of countries file names without an extension for
-  // countries belong to |rect|. |rough| provides fast rough result
-  // or a slower but more precise one.
+  // Returns vector of countries file names without extension for
+  // countries belonging to |rect|. When |rough| is equal to true, the
+  // method is much faster but the result is less precise.
   std::vector<CountryId> GetRegionsCountryIdByRect(m2::RectD const & rect, bool rough) const;
 
   // Returns a list of country ids by a |pt| in mercator.
-  // |closestCoutryIds| is filled with country ids of mwm which covers |pt| or close to it.
-  // |closestCoutryIds| is not filled with country world.mwm country id and with custom mwm.
-  // If |pt| is covered by a sea or a ocean closestCoutryIds may be left empty.
+  // |closestCoutryIds| is filled with country ids of mwms that cover |pt| or are close to it
+  // with the exception of World.mwm and custom user-provided mwms.
+  // The result may be empty, for example if |pt| is somewhere in an ocean.
   void GetRegionsCountryId(m2::PointD const & pt, CountriesVec & closestCoutryIds);
 
-  // Returns info for a region |pt| belongs to.
+  // Fills info for the region |pt| belongs to.
   void GetRegionInfo(m2::PointD const & pt, CountryInfo & info) const;
 
-  // Returns info for a country by id.
+  // Fills info for the country by id.
   void GetRegionInfo(CountryId const & countryId, CountryInfo & info) const;
 
-  // Return limit rects of USA:
+  // Fills limit rects of the USA:
   // 0 - continental part
   // 1 - Alaska
   // 2 - Hawaii
   void CalcUSALimitRect(m2::RectD rects[3]) const;
 
-  // Calculates limit rect for all countries whose name starts with
-  // |prefix|.
-  m2::RectD CalcLimitRect(string const & prefix) const;
-  // Returns limit rect for |countryId| (non-expandable node).
-  // Returns bounding box in mercator coordinates if |countryId| is a country id of non-expandable node
-  // and zero rect otherwise.
+  // Calculates the limit rect for all countries whose names start with |prefix|.
+  m2::RectD CalcLimitRect(std::string const & prefix) const;
+
+  // Returns the limit rect for |countryId| (non-expandable node).
+  // Returns the bounding box in mercator coordinates if |countryId| is a country id of
+  // a non-expandable node and zero rect otherwise.
   m2::RectD GetLimitRectForLeaf(CountryId const & leafCountryId) const;
 
-  // Returns identifiers for all regions matching to correspondent |affiliation|.
-  virtual void GetMatchedRegions(string const & affiliation, TRegionIdSet & regions) const;
+  // Returns identifiers for all regions matching to |affiliation|.
+  virtual void GetMatchedRegions(std::string const & affiliation, RegionIdVec & regions) const;
 
-  // Clears regions cache.
-  inline void ClearCaches() const { ClearCachesImpl(); }
+  // Clears the regions cache.
+  void ClearCaches() const { ClearCachesImpl(); }
 
   void SetAffiliations(Affiliations const * affiliations);
 
 protected:
   CountryInfoGetter() : CountryInfoGetterBase(true /* isSingleMwm */ ) {};
+
   // Invokes |toDo| on each country whose name starts with |prefix|.
   template <typename ToDo>
-  void ForEachCountry(string const & prefix, ToDo && toDo) const;
+  void ForEachCountry(std::string const & prefix, ToDo && toDo) const;
 
   // Clears regions cache.
   virtual void ClearCachesImpl() const = 0;
 
   // Returns true when |rect| intersects a country identified by |id|.
-  virtual bool IsIntersectedByRegionImpl(size_t id, m2::RectD const & rect) const = 0;
+  virtual bool IsIntersectedByRegion(m2::RectD const & rect, size_t id) const = 0;
 
-  // Returns true when the distance from |pt| to country identified by |id| less then |distance|.
+  // Returns true when the distance from |pt| to country identified by |id| is less than |distance|.
   virtual bool IsCloseEnough(size_t id, m2::PointD const & pt, double distance) = 0;
-  // @TODO(bykoianko): consider to get rid of m_countryIndex.
-  // The possibility should be considered.
+
+  // @TODO(bykoianko): consider getting rid of m_countryIndex.
   // Maps all leaf country id (file names) to their indices in m_countries.
-  std::unordered_map<CountryId, TRegionId> m_countryIndex;
+  std::unordered_map<CountryId, RegionId> m_countryIndex;
 
   Affiliations const * m_affiliations = nullptr;
 
-  // Maps country file name without an extension to a country info.
-  std::map<std::string, CountryInfo> m_id2info;
+  // Maps country file name without extension to a country info.
+  std::map<std::string, CountryInfo> m_idToInfo;
 };
 
 // This class reads info about countries from polygons file and
@@ -152,12 +155,14 @@ protected:
 
   // CountryInfoGetter overrides:
   void ClearCachesImpl() const override;
-  bool IsBelongToRegionImpl(size_t id, m2::PointD const & pt) const override;
-  bool IsIntersectedByRegionImpl(size_t id, m2::RectD const & rect) const override;
+  bool BelongsToRegion(m2::PointD const & pt, size_t id) const override;
+  bool IsIntersectedByRegion(m2::RectD const & rect, size_t id) const override;
   bool IsCloseEnough(size_t id, m2::PointD const & pt, double distance) override;
 
-  template <typename TFn>
-  std::result_of_t<TFn(vector<m2::RegionD>)> WithRegion(size_t id, TFn && fn) const;
+  template <typename Fn>
+  std::result_of_t<Fn(std::vector<m2::RegionD>)> WithRegion(size_t id, Fn && fn) const;
+
+  void LoadRegionsFromDisk(size_t id, std::vector<m2::RegionD> & regions) const;
 
   FilesContainerR m_reader;
   mutable base::Cache<uint32_t, std::vector<m2::RegionD>> m_cache;
@@ -176,13 +181,13 @@ public:
   void AddCountry(CountryDef const & country);
 
   // CountryInfoGetter overrides:
-  void GetMatchedRegions(string const & affiliation, TRegionIdSet & regions) const override;
+  void GetMatchedRegions(std::string const & affiliation, RegionIdVec & regions) const override;
 
 protected:
   // CountryInfoGetter overrides:
   void ClearCachesImpl() const override;
-  bool IsBelongToRegionImpl(size_t id, m2::PointD const & pt) const override;
-  bool IsIntersectedByRegionImpl(size_t id, m2::RectD const & rect) const override;
+  bool BelongsToRegion(m2::PointD const & pt, size_t id) const override;
+  bool IsIntersectedByRegion(m2::RectD const & rect, size_t id) const override;
   bool IsCloseEnough(size_t id, m2::PointD const & pt, double distance) override;
 };
 }  // namespace storage
diff --git a/storage/country_info_reader_light.cpp b/storage/country_info_reader_light.cpp
index 2db93c2a42..02354fb534 100644
--- a/storage/country_info_reader_light.cpp
+++ b/storage/country_info_reader_light.cpp
@@ -17,7 +17,6 @@
 #include "base/string_utils.hpp"
 
 #include <chrono>
-#include <string>
 #include <utility>
 #include <vector>
 
@@ -46,20 +45,24 @@ CountryInfoReader::CountryInfoReader()
   m_nameGetter.SetLocale(languages::GetCurrentTwine());
 }
 
-bool CountryInfoReader::IsBelongToRegionImpl(size_t id, m2::PointD const & pt) const
+void CountryInfoReader::LoadRegionsFromDisk(size_t id, std::vector<m2::RegionD> & regions) const
 {
-  // Load regions from file.
+  regions.clear();
   ReaderSource<ModelReaderPtr> src(m_reader->GetReader(strings::to_string(id)));
 
   uint32_t const count = ReadVarUint<uint32_t>(src);
-  std::vector<m2::RegionD> regions;
-
   for (size_t i = 0; i < count; ++i)
   {
     std::vector<m2::PointD> points;
     serial::LoadOuterPath(src, serial::GeometryCodingParams(), points);
-    regions.emplace_back(move(points));
+    regions.emplace_back(std::move(points));
   }
+}
+
+bool CountryInfoReader::BelongsToRegion(m2::PointD const & pt, size_t id) const
+{
+  std::vector<m2::RegionD> regions;
+  LoadRegionsFromDisk(id, regions);
 
   for (auto const & region : regions)
   {
diff --git a/storage/country_info_reader_light.hpp b/storage/country_info_reader_light.hpp
index a4e48846a8..2a2af911c9 100644
--- a/storage/country_info_reader_light.hpp
+++ b/storage/country_info_reader_light.hpp
@@ -7,9 +7,12 @@
 #include "coding/file_container.hpp"
 
 #include "geometry/point2d.hpp"
+#include "geometry/region2d.hpp"
 
-#include <cstdint>
+#include <cstddef>
 #include <memory>
+#include <string>
+#include <vector>
 
 namespace lightweight
 {
@@ -27,7 +30,10 @@ public:
   Info GetMwmInfo(m2::PointD const & pt) const;
 
 protected:
-  bool IsBelongToRegionImpl(size_t id, m2::PointD const & pt) const override;
+  void LoadRegionsFromDisk(size_t id, std::vector<m2::RegionD> & regions) const;
+
+  // storage::CountryInfoGetterBase overrides:
+  bool BelongsToRegion(m2::PointD const & pt, size_t id) const override;
 
 private:
   std::unique_ptr<FilesContainerR> m_reader;
diff --git a/storage/country_polygon.hpp b/storage/country_polygon.hpp
index 992a887258..cfba7c06d4 100644
--- a/storage/country_polygon.hpp
+++ b/storage/country_polygon.hpp
@@ -7,25 +7,28 @@
 #include "coding/read_write_utils.hpp"
 #include "coding/varint.hpp"
 
+#include <cstdint>
+#include <utility>
+
 namespace storage
 {
-template <class TSource>
-void Read(TSource & src, CountryDef & p)
+template <class Source>
+void Read(Source & src, CountryDef & p)
 {
   rw::Read(src, p.m_countryId);
 
-  pair<int64_t, int64_t> r;
+  std::pair<int64_t, int64_t> r;
   r.first = ReadVarInt<int64_t>(src);
   r.second = ReadVarInt<int64_t>(src);
   p.m_rect = Int64ToRectObsolete(r, serial::GeometryCodingParams().GetCoordBits());
 }
 
-template <class TSink>
-void Write(TSink & sink, CountryDef const & p)
+template <class Sink>
+void Write(Sink & sink, CountryDef const & p)
 {
   rw::Write(sink, p.m_countryId);
 
-  pair<int64_t, int64_t> const r =
+  std::pair<int64_t, int64_t> const r =
       RectToInt64Obsolete(p.m_rect, serial::GeometryCodingParams().GetCoordBits());
 
   WriteVarInt(sink, r.first);
diff --git a/storage/storage_tests/CMakeLists.txt b/storage/storage_tests/CMakeLists.txt
index 01581121a6..80528bb434 100644
--- a/storage/storage_tests/CMakeLists.txt
+++ b/storage/storage_tests/CMakeLists.txt
@@ -4,8 +4,8 @@ add_definitions("-DOMIM_UNIT_TEST_WITH_QT_EVENT_LOOP")
 
 set(
   SRC
-  country_info_getter_test.cpp
-  country_name_getter_test.cpp
+  country_info_getter_tests.cpp
+  country_name_getter_tests.cpp
   fake_map_files_downloader.cpp
   fake_map_files_downloader.hpp
   helpers.cpp
diff --git a/storage/storage_tests/country_info_getter_test.cpp b/storage/storage_tests/country_info_getter_tests.cpp
index 2dc4e9d64c..17edfdda10 100644
--- a/storage/storage_tests/country_info_getter_test.cpp
+++ b/storage/storage_tests/country_info_getter_tests.cpp
@@ -2,8 +2,8 @@
 
 #include "storage/storage_tests/helpers.hpp"
 
-#include "storage/country_info_getter.hpp"
 #include "storage/country.hpp"
+#include "storage/country_info_getter.hpp"
 #include "storage/storage.hpp"
 
 #include "geometry/mercator.hpp"
@@ -15,11 +15,14 @@
 
 #include "base/logging.hpp"
 
-#include "std/unique_ptr.hpp"
-#include "std/vector.hpp"
-
+#include <map>
+#include <memory>
+#include <random>
+#include <string>
+#include <vector>
 
 using namespace storage;
+using namespace std;
 
 namespace
 {
@@ -62,8 +65,8 @@ UNIT_TEST(CountryInfoGetter_GetRegionsCountryIdByRect_Smoke)
 
   // Several countries.
   m2::PointD const halfSize2 = m2::PointD(5.0, 5.0);
-  auto const countries2 = getter->GetRegionsCountryIdByRect(m2::RectD(p - halfSize2, p + halfSize2),
-                                                            false /* rough */);
+  auto const countries2 =
+      getter->GetRegionsCountryIdByRect(m2::RectD(p - halfSize2, p + halfSize2), false /* rough */);
   auto const expected = vector<storage::CountryId>{
       "Belarus", "Latvia", "Lithuania", "Poland", "Russia_Central", "Russia_Northwestern",
       "Ukraine"};
@@ -125,11 +128,11 @@ UNIT_TEST(CountryInfoGetter_SomeRects)
   m2::RectD rects[3];
   getter->CalcUSALimitRect(rects);
 
-  LOG(LINFO, ("USA Continental: ", rects[0]));
-  LOG(LINFO, ("Alaska: ", rects[1]));
-  LOG(LINFO, ("Hawaii: ", rects[2]));
+  LOG(LINFO, ("USA Continental:", rects[0]));
+  LOG(LINFO, ("Alaska:", rects[1]));
+  LOG(LINFO, ("Hawaii:", rects[2]));
 
-  LOG(LINFO, ("Canada: ", getter->CalcLimitRect("Canada_")));
+  LOG(LINFO, ("Canada:", getter->CalcLimitRect("Canada_")));
 }
 
 UNIT_TEST(CountryInfoGetter_HitsInRadius)
@@ -149,7 +152,8 @@ UNIT_TEST(CountryInfoGetter_HitsOnLongLine)
   CountriesVec results;
   getter->GetRegionsCountryId(MercatorBounds::FromLatLon(62.2507, -102.0753), results);
   TEST_EQUAL(results.size(), 2, ());
-  TEST(find(results.begin(), results.end(), "Canada_Northwest Territories_East") != results.end(), ());
+  TEST(find(results.begin(), results.end(), "Canada_Northwest Territories_East") != results.end(),
+       ());
   TEST(find(results.begin(), results.end(), "Canada_Nunavut_South") != results.end(), ());
 }
 
@@ -159,7 +163,8 @@ UNIT_TEST(CountryInfoGetter_HitsInTheMiddleOfNowhere)
   CountriesVec results;
   getter->GetRegionsCountryId(MercatorBounds::FromLatLon(62.2900, -103.9423), results);
   TEST_EQUAL(results.size(), 1, ());
-  TEST(find(results.begin(), results.end(), "Canada_Northwest Territories_East") != results.end(), ());
+  TEST(find(results.begin(), results.end(), "Canada_Northwest Territories_East") != results.end(),
+       ());
 }
 
 UNIT_TEST(CountryInfoGetter_GetLimitRectForLeafSingleMwm)
diff --git a/storage/storage_tests/country_name_getter_test.cpp b/storage/storage_tests/country_name_getter_tests.cpp
index 32c8e2492f..32c8e2492f 100644
--- a/storage/storage_tests/country_name_getter_test.cpp
+++ b/storage/storage_tests/country_name_getter_tests.cpp