Review fixes.

17 files changed, 127 insertions, 77 deletions

diff --git a/base/base_tests/CMakeLists.txt b/base/base_tests/CMakeLists.txt
index ad29bf1f97..8cc8ddf949 100644
--- a/base/base_tests/CMakeLists.txt
+++ b/base/base_tests/CMakeLists.txt
@@ -38,6 +38,7 @@ set(
   timegm_test.cpp
   timer_test.cpp
   uni_string_dfa_test.cpp
+  visitor_tests.cpp
   worker_thread_tests.cpp
 )
 
diff --git a/base/base_tests/base_tests.pro b/base/base_tests/base_tests.pro
index 501e9d1e3a..32c72bb91b 100644
--- a/base/base_tests/base_tests.pro
+++ b/base/base_tests/base_tests.pro
@@ -48,6 +48,7 @@ SOURCES += \
   timegm_test.cpp \
   timer_test.cpp \
   uni_string_dfa_test.cpp \
+  visitor_tests.cpp \
   worker_thread_tests.cpp \
 
 HEADERS +=
diff --git a/base/base_tests/visitor_tests.cpp b/base/base_tests/visitor_tests.cpp
new file mode 100644
index 0000000000..d5e4eb1380
--- /dev/null
+++ b/base/base_tests/visitor_tests.cpp
@@ -0,0 +1,45 @@
+#include "testing/testing.hpp"
+
+#include "base/visitor.hpp"
+
+#include <string>
+
+using namespace std;
+
+namespace
+{
+struct Foo
+{
+  DECLARE_VISITOR()
+  DECLARE_DEBUG_PRINT(Foo)
+};
+
+struct Bar
+{
+  Bar() = default;
+  Bar(int d, string const & s) : m_d(d), m_s(s) {}
+
+  DECLARE_VISITOR(visitor(m_d), visitor(m_s, "string"))
+  DECLARE_DEBUG_PRINT(Bar)
+
+  int m_d = 0;
+  string m_s;
+};
+
+UNIT_TEST(DebugPrintVisitor_Smoke)
+{
+  {
+    Foo foo;
+    TEST_EQUAL(DebugPrint(foo), "Foo []", ());
+  }
+
+  {
+    Bar bar;
+    TEST_EQUAL(DebugPrint(bar), "Bar [0, string: ]", ());
+
+    bar.m_d = 7;
+    bar.m_s = "Hello, World!";
+    TEST_EQUAL(DebugPrint(bar), "Bar [7, string: Hello, World!]", ());
+  }
+}
+}  // namespace
diff --git a/base/visitor.hpp b/base/visitor.hpp
index 8f3f0f73a8..315ef1a962 100644
--- a/base/visitor.hpp
+++ b/base/visitor.hpp
@@ -1,5 +1,36 @@
 #pragma once
 
+#include <sstream>
+#include <string>
+
+namespace base
+{
+class DebugPrintVisitor
+{
+public:
+  DebugPrintVisitor(std::string const & name) : m_name(name) {}
+
+  template <typename T>
+  void operator()(T const & t, char const * name = nullptr)
+  {
+    if (!m_empty)
+      m_os << ", ";
+    m_empty = false;
+
+    if (name)
+      m_os << name << ": ";
+    m_os << DebugPrint(t);
+  }
+
+  std::string ToString() const { return m_name + " [" + m_os.str() + "]"; }
+
+private:
+  std::string m_name;
+  bool m_empty = true;
+  std::ostringstream m_os;
+};
+}  // namespace base
+
 #define DECLARE_VISITOR(...)          \
   template <typename Visitor>         \
   void Visit(Visitor & visitor)       \
@@ -11,3 +42,11 @@
   {                                   \
     __VA_ARGS__;                      \
   }
+
+#define DECLARE_DEBUG_PRINT(className)               \
+  friend std::string DebugPrint(className const & c) \
+  {                                                  \
+    base::DebugPrintVisitor visitor(#className);     \
+    c.Visit(visitor);                                \
+    return visitor.ToString();                       \
+  }
diff --git a/geometry/bounding_box.cpp b/geometry/bounding_box.cpp
index d968239e2f..5a3c5dee69 100644
--- a/geometry/bounding_box.cpp
+++ b/geometry/bounding_box.cpp
@@ -1,7 +1,6 @@
 #include "geometry/bounding_box.hpp"
 
 #include <algorithm>
-#include <sstream>
 
 using namespace std;
 
@@ -25,14 +24,4 @@ bool BoundingBox::HasPoint(double x, double y) const
 {
   return x >= m_min.x && x <= m_max.x && y >= m_min.y && y <= m_max.y;
 }
-
-string DebugPrint(BoundingBox const & bbox)
-{
-  ostringstream os;
-  os << "BoundingBox [ ";
-  os << "min: " << DebugPrint(bbox.Min()) << ", ";
-  os << "max: " << DebugPrint(bbox.Max());
-  os << " ]";
-  return os.str();
-}
 }  // namespace m2
diff --git a/geometry/bounding_box.hpp b/geometry/bounding_box.hpp
index 2e9209cfab..eaf48a85b6 100644
--- a/geometry/bounding_box.hpp
+++ b/geometry/bounding_box.hpp
@@ -5,7 +5,6 @@
 #include "base/visitor.hpp"
 
 #include <limits>
-#include <string>
 #include <vector>
 
 namespace m2
@@ -40,7 +39,8 @@ public:
     return m_min == rhs.m_min && m_max == rhs.m_max;
   }
 
-  DECLARE_VISITOR(visitor(m_min), visitor(m_max))
+  DECLARE_VISITOR(visitor(m_min, "min"), visitor(m_max, "max"))
+  DECLARE_DEBUG_PRINT(BoundingBox)
 
 private:
   static_assert(std::numeric_limits<double>::has_infinity, "");
@@ -50,6 +50,4 @@ private:
   m2::PointD m_min = m2::PointD(kPositiveInfinity, kPositiveInfinity);
   m2::PointD m_max = m2::PointD(kNegativeInfinity, kNegativeInfinity);
 };
-
-std::string DebugPrint(BoundingBox const & bbox);
 }  // namespace m2
diff --git a/geometry/calipers_box.cpp b/geometry/calipers_box.cpp
index a5ad7fe27b..ddbb42bc11 100644
--- a/geometry/calipers_box.cpp
+++ b/geometry/calipers_box.cpp
@@ -147,6 +147,4 @@ bool CalipersBox::HasPoint(PointD const & p) const
   }
   return true;
 }
-
-string DebugPrint(CalipersBox const & cbox) { return "CalipersBox " + ::DebugPrint(cbox.Points()); }
 }  // namespace m2
diff --git a/geometry/calipers_box.hpp b/geometry/calipers_box.hpp
index 6cf7299dc5..d66e354186 100644
--- a/geometry/calipers_box.hpp
+++ b/geometry/calipers_box.hpp
@@ -4,7 +4,6 @@
 
 #include "base/visitor.hpp"
 
-#include <string>
 #include <vector>
 
 namespace m2
@@ -19,7 +18,7 @@ class CalipersBox
 {
 public:
   CalipersBox() = default;
-  CalipersBox(std::vector<PointD> const & points);
+  explicit CalipersBox(std::vector<PointD> const & points);
 
   std::vector<PointD> const & Points() const { return m_points; }
 
@@ -28,11 +27,10 @@ public:
 
   bool operator==(CalipersBox const & rhs) const { return m_points == rhs.m_points; }
 
-  DECLARE_VISITOR(visitor(m_points))
+  DECLARE_VISITOR(visitor(m_points, "points"))
+  DECLARE_DEBUG_PRINT(CalipersBox)
 
 private:
   std::vector<PointD> m_points;
 };
-
-std::string DebugPrint(CalipersBox const & cbox);
 }  // namespace m2
diff --git a/geometry/diamond_box.cpp b/geometry/diamond_box.cpp
index 07fc21e1a2..60e58fabe0 100644
--- a/geometry/diamond_box.cpp
+++ b/geometry/diamond_box.cpp
@@ -9,9 +9,4 @@ DiamondBox::DiamondBox(vector<PointD> const & points)
   for (auto const & p : points)
     Add(p);
 }
-
-string DebugPrint(DiamondBox const & dbox)
-{
-  return "DiamondBox [ " + ::DebugPrint(dbox.Points()) + " ]";
-}
 }  // namespace m2
diff --git a/geometry/diamond_box.hpp b/geometry/diamond_box.hpp
index 436eca4009..341249c591 100644
--- a/geometry/diamond_box.hpp
+++ b/geometry/diamond_box.hpp
@@ -5,7 +5,6 @@
 
 #include "base/visitor.hpp"
 
-#include <string>
 #include <vector>
 
 namespace m2
@@ -16,7 +15,7 @@ class DiamondBox
 {
 public:
   DiamondBox() = default;
-  DiamondBox(std::vector<PointD> const & points);
+  explicit DiamondBox(std::vector<PointD> const & points);
 
   void Add(PointD const & p) { return Add(p.x, p.y); }
   void Add(double x, double y) { return m_box.Add(x + y, x - y); }
@@ -34,7 +33,8 @@ public:
 
   bool operator==(DiamondBox const & rhs) const { return m_box == rhs.m_box; }
 
-  DECLARE_VISITOR(visitor(m_box))
+  DECLARE_VISITOR(visitor(Points(), "points"))
+  DECLARE_DEBUG_PRINT(DiamondBox)
 
 private:
   static m2::PointD ToOrig(m2::PointD const & p)
@@ -44,6 +44,4 @@ private:
 
   BoundingBox m_box;
 };
-
-std::string DebugPrint(DiamondBox const & dbox);
 }  // namespace m2
diff --git a/indexer/CMakeLists.txt b/indexer/CMakeLists.txt
index 4cba9f5bf4..45328415c5 100644
--- a/indexer/CMakeLists.txt
+++ b/indexer/CMakeLists.txt
@@ -6,20 +6,20 @@ set(
   SRC
   altitude_loader.cpp
   altitude_loader.hpp
-  boundary_boxes.hpp
-  boundary_boxes_serdes.hpp
-  categories_holder_loader.cpp
   categories_holder.cpp
   categories_holder.hpp
+  categories_holder_loader.cpp
   categories_index.cpp
   categories_index.hpp
   cell_coverer.hpp
   cell_id.hpp
   centers_table.cpp
   centers_table.hpp
+  cities_boundaries_serdes.hpp
+  city_boundary.hpp
+  classificator.cpp
   classificator_loader.cpp
   classificator_loader.hpp
-  classificator.cpp
   classificator.hpp
   coding_params.cpp
   coding_params.hpp
diff --git a/indexer/boundary_boxes_serdes.hpp b/indexer/cities_boundaries_serdes.hpp
index 3158ba6a02..913802ddad 100644
--- a/indexer/boundary_boxes_serdes.hpp
+++ b/indexer/cities_boundaries_serdes.hpp
@@ -24,7 +24,7 @@
 namespace indexer
 {
 template <typename Sink>
-class BoundaryBoxesEncoder
+class CityBoundaryEncoder
 {
 public:
   struct Visitor
@@ -149,25 +149,25 @@ public:
     m2::PointU m_last;
   };
 
-  BoundaryBoxesEncoder(Sink & sink, serial::CodingParams const & params)
+  CityBoundaryEncoder(Sink & sink, serial::CodingParams const & params)
     : m_sink(sink), m_visitor(sink, params)
   {
   }
 
-  void operator()(std::vector<std::vector<BoundaryBoxes>> const & boxes)
+  void operator()(std::vector<std::vector<CityBoundary>> const & boundaries)
   {
-    WriteVarUint(m_sink, boxes.size());
+    WriteVarUint(m_sink, boundaries.size());
 
     {
       BitWriter<Sink> writer(m_sink);
-      for (auto const & bs : boxes)
+      for (auto const & bs : boundaries)
       {
         CHECK(!bs.empty(), ());
         coding::GammaCoder::Encode(writer, bs.size());
       }
     }
 
-    for (auto const & bs : boxes)
+    for (auto const & bs : boundaries)
     {
       for (auto const & b : bs)
         m_visitor(b);
@@ -181,7 +181,7 @@ private:
 };
 
 template <typename Source>
-class BoundaryBoxesDecoder
+class CityBoundaryDecoder
 {
 public:
   struct Visitor
@@ -285,26 +285,26 @@ public:
     m2::PointU m_last;
   };
 
-  BoundaryBoxesDecoder(Source & source, serial::CodingParams const & params)
+  CityBoundaryDecoder(Source & source, serial::CodingParams const & params)
     : m_source(source), m_visitor(source, params)
   {
   }
 
-  void operator()(std::vector<std::vector<BoundaryBoxes>> & boxes)
+  void operator()(std::vector<std::vector<CityBoundary>> & boundaries)
   {
     auto const size = ReadVarUint<uint64_t>(m_source);
-    boxes.resize(size);
+    boundaries.resize(size);
 
     {
       BitReader<Source> reader(m_source);
-      for (auto & bs : boxes)
+      for (auto & bs : boundaries)
       {
         auto const size = coding::GammaCoder::Decode(reader);
         bs.resize(size);
       }
     }
 
-    for (auto & bs : boxes)
+    for (auto & bs : boundaries)
     {
       for (auto & b : bs)
         m_visitor(b);
diff --git a/indexer/boundary_boxes.hpp b/indexer/city_boundary.hpp
index 381bddc821..920a7e202a 100644
--- a/indexer/boundary_boxes.hpp
+++ b/indexer/city_boundary.hpp
@@ -7,17 +7,15 @@
 
 #include "base/visitor.hpp"
 
-#include <sstream>
-#include <string>
 #include <vector>
 
 namespace indexer
 {
-struct BoundaryBoxes
+struct CityBoundary
 {
-  BoundaryBoxes() = default;
+  CityBoundary() = default;
 
-  BoundaryBoxes(std::vector<m2::PointD> const & ps) : m_bbox(ps), m_cbox(ps), m_dbox(ps) {}
+  explicit CityBoundary(std::vector<m2::PointD> const & ps) : m_bbox(ps), m_cbox(ps), m_dbox(ps) {}
 
   bool HasPoint(m2::PointD const & p) const
   {
@@ -26,25 +24,16 @@ struct BoundaryBoxes
 
   bool HasPoint(double x, double y) const { return HasPoint(m2::PointD(x, y)); }
 
-  bool operator==(BoundaryBoxes const & rhs) const
+  bool operator==(CityBoundary const & rhs) const
   {
     return m_bbox == rhs.m_bbox && m_cbox == rhs.m_cbox && m_dbox == rhs.m_dbox;
   }
 
   DECLARE_VISITOR(visitor(m_bbox), visitor(m_cbox), visitor(m_dbox))
+  DECLARE_DEBUG_PRINT(CityBoundary)
 
   m2::BoundingBox m_bbox;
   m2::CalipersBox m_cbox;
   m2::DiamondBox m_dbox;
 };
-
-inline std::string DebugPrint(BoundaryBoxes const & boxes)
-{
-  std::ostringstream os;
-  os << "BoundaryBoxes [ ";
-  os << DebugPrint(boxes.m_bbox) << ", ";
-  os << DebugPrint(boxes.m_cbox) << ", ";
-  os << DebugPrint(boxes.m_dbox) << " ]";
-  return os.str();
-}
 }  // namespace indexer
diff --git a/indexer/indexer.pro b/indexer/indexer.pro
index 03a3d6fea8..63f8920d4a 100644
--- a/indexer/indexer.pro
+++ b/indexer/indexer.pro
@@ -64,13 +64,13 @@ SOURCES += \
 
 HEADERS += \
     altitude_loader.hpp \
-    boundary_boxes.hpp \
-    boundary_boxes_serdes.hpp \
     categories_holder.hpp \
     categories_index.hpp \
     cell_coverer.hpp \
     cell_id.hpp \
     centers_table.hpp \
+    cities_boundaries_serdes.hpp \
+    city_boundary.hpp \
     classificator.hpp \
     classificator_loader.hpp \
     coding_params.hpp \
diff --git a/indexer/indexer_tests/CMakeLists.txt b/indexer/indexer_tests/CMakeLists.txt
index 43e299568e..101b72eab3 100644
--- a/indexer/indexer_tests/CMakeLists.txt
+++ b/indexer/indexer_tests/CMakeLists.txt
@@ -2,12 +2,12 @@ project(indexer_tests)
 
 set(
   SRC
-  boundary_boxes_serdes_tests.cpp
   categories_test.cpp
   cell_coverer_test.cpp
   cell_id_test.cpp
   centers_table_test.cpp
   checker_test.cpp
+  cities_boundaries_serdes_tests.cpp
   drules_selector_parser_test.cpp
   editable_map_object_test.cpp
   feature_metadata_test.cpp
diff --git a/indexer/indexer_tests/boundary_boxes_serdes_tests.cpp b/indexer/indexer_tests/cities_boundaries_serdes_tests.cpp
index 1a5511d5bf..7bf99d2a00 100644
--- a/indexer/indexer_tests/boundary_boxes_serdes_tests.cpp
+++ b/indexer/indexer_tests/cities_boundaries_serdes_tests.cpp
@@ -1,7 +1,7 @@
 #include "testing/testing.hpp"
 
-#include "indexer/boundary_boxes.hpp"
-#include "indexer/boundary_boxes_serdes.hpp"
+#include "indexer/city_boundary.hpp"
+#include "indexer/cities_boundaries_serdes.hpp"
 #include "indexer/coding_params.hpp"
 
 #include "coding/reader.hpp"
@@ -20,7 +20,7 @@ using namespace std;
 
 namespace
 {
-using Boundary = vector<BoundaryBoxes>;
+using Boundary = vector<CityBoundary>;
 using Boundaries = vector<Boundary>;
 
 void TestEqual(BoundingBox const & lhs, BoundingBox const & rhs, double eps)
@@ -30,6 +30,7 @@ void TestEqual(BoundingBox const & lhs, BoundingBox const & rhs, double eps)
 }
 
 void TestEqual(CalipersBox const & lhs, CalipersBox const & rhs, double eps) {}
+
 void TestEqual(DiamondBox const & lhs, DiamondBox const & rhs, double eps)
 {
   auto const lps = lhs.Points();
@@ -37,12 +38,10 @@ void TestEqual(DiamondBox const & lhs, DiamondBox const & rhs, double eps)
   TEST_EQUAL(lps.size(), 4, (lhs));
   TEST_EQUAL(rps.size(), 4, (rhs));
   for (size_t i = 0; i < 4; ++i)
-  {
     TEST(AlmostEqualAbs(lps[i], rps[i], eps), (lhs, rhs));
-  }
 }
 
-void TestEqual(BoundaryBoxes const & lhs, BoundaryBoxes const & rhs, double eps)
+void TestEqual(CityBoundary const & lhs, CityBoundary const & rhs, double eps)
 {
   TestEqual(lhs.m_bbox, rhs.m_bbox, eps);
   TestEqual(lhs.m_cbox, rhs.m_cbox, eps);
@@ -68,7 +67,7 @@ Boundaries EncodeDecode(Boundaries const & boundaries, CodingParams const & para
   vector<uint8_t> buffer;
   {
     MemWriter<decltype(buffer)> sink(buffer);
-    BoundaryBoxesEncoder<decltype(sink)> encoder(sink, params);
+    CityBoundaryEncoder<decltype(sink)> encoder(sink, params);
     encoder(boundaries);
   }
 
@@ -76,7 +75,7 @@ Boundaries EncodeDecode(Boundaries const & boundaries, CodingParams const & para
     Boundaries boundaries;
     MemReader reader(buffer.data(), buffer.size());
     NonOwningReaderSource source(reader);
-    BoundaryBoxesDecoder<decltype(source)> decoder(source, params);
+    CityBoundaryDecoder<decltype(source)> decoder(source, params);
     decoder(boundaries);
     return boundaries;
   }
@@ -88,7 +87,7 @@ void TestEncodeDecode(Boundaries const & expected, CodingParams const & params,
   TestEqual(expected, actual, eps);
 }
 
-UNIT_TEST(BoundaryBoxesSerDes_Smoke)
+UNIT_TEST(CitiesBoundariesSerDes_Smoke)
 {
   CodingParams const params(19 /* coordBits */, PointD(MercatorBounds::minX, MercatorBounds::minY));
   double const kEps = 1e-3;
diff --git a/indexer/indexer_tests/indexer_tests.pro b/indexer/indexer_tests/indexer_tests.pro
index 1220c484c2..9485d04fac 100644
--- a/indexer/indexer_tests/indexer_tests.pro
+++ b/indexer/indexer_tests/indexer_tests.pro
@@ -28,12 +28,12 @@ HEADERS += \
 
 SOURCES += \
     ../../testing/testingmain.cpp \
-    boundary_boxes_serdes_tests.cpp \
     categories_test.cpp \
     cell_coverer_test.cpp \
     cell_id_test.cpp \
     centers_table_test.cpp \
     checker_test.cpp \
+    cities_boundaries_serdes_tests.cpp \
     drules_selector_parser_test.cpp \
     editable_map_object_test.cpp \
     feature_metadata_test.cpp \