[indexer] Disallow copy and move of FeatureType, remove FeatureType::ParseEverything().

10 files changed, 80 insertions, 110 deletions

diff --git a/indexer/data_source.cpp b/indexer/data_source.cpp
index dcfb77b827..17510a83d8 100644
--- a/indexer/data_source.cpp
+++ b/indexer/data_source.cpp
@@ -79,7 +79,7 @@ private:
 
 void ReadFeatureType(function<void(FeatureType &)> const & fn, FeatureSource & src, uint32_t index)
 {
-  FeatureType feature;
+  unique_ptr<FeatureType> ft;
   switch (src.GetFeatureStatus(index))
   {
   case FeatureStatus::Deleted:
@@ -87,16 +87,17 @@ void ReadFeatureType(function<void(FeatureType &)> const & fn, FeatureSource & s
   case FeatureStatus::Created:
   case FeatureStatus::Modified:
   {
-    VERIFY(src.GetModifiedFeature(index, feature), ());
+    ft = src.GetModifiedFeature(index);
     break;
   }
   case FeatureStatus::Untouched:
   {
-    src.GetOriginalFeature(index, feature);
+    ft = src.GetOriginalFeature(index);
     break;
   }
   }
-  fn(feature);
+  CHECK(ft, ());
+  fn(*ft);
 }
 }  //  namespace
 
@@ -117,45 +118,33 @@ bool FeaturesLoaderGuard::IsWorld() const
   return m_handle.GetValue<MwmValue>()->GetHeader().GetType() == feature::DataHeader::world;
 }
 
-unique_ptr<FeatureType> FeaturesLoaderGuard::GetOriginalFeatureByIndex(uint32_t index) const
-{
-  auto feature = make_unique<FeatureType>();
-  if (GetOriginalFeatureByIndex(index, *feature))
-    return feature;
-
-  return {};
-}
-
 unique_ptr<FeatureType> FeaturesLoaderGuard::GetOriginalOrEditedFeatureByIndex(uint32_t index) const
 {
-  auto feature = make_unique<FeatureType>();
   if (!m_handle.IsAlive())
     return {};
 
   ASSERT_NOT_EQUAL(m_source->GetFeatureStatus(index), FeatureStatus::Created, ());
-  if (GetFeatureByIndex(index, *feature))
-    return feature;
-
-  return {};
+  return GetFeatureByIndex(index);
 }
 
-WARN_UNUSED_RESULT bool FeaturesLoaderGuard::GetFeatureByIndex(uint32_t index,
-                                                               FeatureType & ft) const
+unique_ptr<FeatureType> FeaturesLoaderGuard::GetFeatureByIndex(uint32_t index) const
 {
   if (!m_handle.IsAlive())
-    return false;
+    return {};
 
   ASSERT_NOT_EQUAL(FeatureStatus::Deleted, m_source->GetFeatureStatus(index),
                    ("Deleted feature was cached. It should not be here. Please review your code."));
-  if (m_source->GetModifiedFeature(index, ft))
-    return true;
-  return GetOriginalFeatureByIndex(index, ft);
+
+  auto ft = m_source->GetModifiedFeature(index);
+  if (ft)
+    return ft;
+
+  return GetOriginalFeatureByIndex(index);
 }
 
-WARN_UNUSED_RESULT bool FeaturesLoaderGuard::GetOriginalFeatureByIndex(uint32_t index,
-                                                                       FeatureType & ft) const
+unique_ptr<FeatureType> FeaturesLoaderGuard::GetOriginalFeatureByIndex(uint32_t index) const
 {
-  return m_handle.IsAlive() ? m_source->GetOriginalFeature(index, ft) : false;
+  return m_handle.IsAlive() ? m_source->GetOriginalFeature(index) : nullptr;
 }
 
 // DataSource ----------------------------------------------------------------------------------
@@ -310,12 +299,13 @@ void DataSource::ReadFeatures(FeatureCallback const & fn, vector<FeatureID> cons
         ASSERT_NOT_EQUAL(
             FeatureStatus::Deleted, fts,
             ("Deleted feature was cached. It should not be here. Please review your code."));
-        FeatureType featureType;
+        unique_ptr<FeatureType> ft;
         if (fts == FeatureStatus::Modified || fts == FeatureStatus::Created)
-          VERIFY(src->GetModifiedFeature(fidIter->m_index, featureType), ());
+          ft = src->GetModifiedFeature(fidIter->m_index);
         else
-          src->GetOriginalFeature(fidIter->m_index, featureType);
-        fn(featureType);
+          ft = src->GetOriginalFeature(fidIter->m_index);
+        CHECK(ft, ());
+        fn(*ft);
       } while (++fidIter != endIter && id == fidIter->m_mwmId);
     }
     else
diff --git a/indexer/data_source.hpp b/indexer/data_source.hpp
index a186680f6c..db97492622 100644
--- a/indexer/data_source.hpp
+++ b/indexer/data_source.hpp
@@ -100,12 +100,11 @@ public:
   MwmSet::MwmId const & GetId() const { return m_handle.GetId(); }
   std::string GetCountryFileName() const;
   bool IsWorld() const;
+  /// Editor core only method, to get 'untouched', original version of feature.
   std::unique_ptr<FeatureType> GetOriginalFeatureByIndex(uint32_t index) const;
   std::unique_ptr<FeatureType> GetOriginalOrEditedFeatureByIndex(uint32_t index) const;
   /// Everyone, except Editor core, should use this method.
-  WARN_UNUSED_RESULT bool GetFeatureByIndex(uint32_t index, FeatureType & ft) const;
-  /// Editor core only method, to get 'untouched', original version of feature.
-  WARN_UNUSED_RESULT bool GetOriginalFeatureByIndex(uint32_t index, FeatureType & ft) const;
+  std::unique_ptr<FeatureType> GetFeatureByIndex(uint32_t index) const;
   size_t GetNumFeatures() const { return m_source->GetNumFeatures(); }
 
 private:
diff --git a/indexer/feature.cpp b/indexer/feature.cpp
index 89ea2333e8..6cc25b7501 100644
--- a/indexer/feature.cpp
+++ b/indexer/feature.cpp
@@ -180,67 +180,64 @@ uint8_t ReadByte(TSource & src)
 }
 }  // namespace
 
-// static
-FeatureType FeatureType::ConstructFromMapObject(osm::MapObject const & emo)
+FeatureType::FeatureType(SharedLoadInfo const * loadInfo, Buffer buffer)
+{
+  CHECK(loadInfo, ());
+  m_loadInfo = loadInfo;
+  m_data = buffer;
+  m_header = Header(m_data);
+
+  m_offsets.Reset();
+  m_ptsSimpMask = 0;
+  m_limitRect = m2::RectD::GetEmptyRect();
+  m_parsed.Reset();
+  m_innerStats.MakeZero();
+}
+
+FeatureType::FeatureType(osm::MapObject const & emo)
 {
-  FeatureType ft;
   uint8_t const geomType = emo.GetGeomType();
-  ft.m_limitRect.MakeEmpty();
+  m_limitRect.MakeEmpty();
 
   switch (geomType)
   {
   case feature::GEOM_POINT:
-    ft.m_center = emo.GetMercator();
-    ft.m_limitRect.Add(ft.m_center);
+    m_center = emo.GetMercator();
+    m_limitRect.Add(m_center);
     break;
   case feature::GEOM_LINE:
-    ft.m_points = Points(emo.GetPoints().begin(), emo.GetPoints().end());
-    for (auto const & p : ft.m_points)
-      ft.m_limitRect.Add(p);
+    m_points = Points(emo.GetPoints().begin(), emo.GetPoints().end());
+    for (auto const & p : m_points)
+      m_limitRect.Add(p);
     break;
   case feature::GEOM_AREA:
-    ft.m_triangles = Points(emo.GetTriangesAsPoints().begin(), emo.GetTriangesAsPoints().end());
-    for (auto const & p : ft.m_triangles)
-      ft.m_limitRect.Add(p);
+    m_triangles = Points(emo.GetTriangesAsPoints().begin(), emo.GetTriangesAsPoints().end());
+    for (auto const & p : m_triangles)
+      m_limitRect.Add(p);
     break;
   }
 
-  ft.m_parsed.m_points = ft.m_parsed.m_triangles = true;
+  m_parsed.m_points = m_parsed.m_triangles = true;
 
-  ft.m_params.name = emo.GetNameMultilang();
+  m_params.name = emo.GetNameMultilang();
   string const & house = emo.GetHouseNumber();
   if (house.empty())
-    ft.m_params.house.Clear();
+    m_params.house.Clear();
   else
-    ft.m_params.house.Set(house);
-  ft.m_parsed.m_common = true;
+    m_params.house.Set(house);
+  m_parsed.m_common = true;
 
-  ft.m_metadata = emo.GetMetadata();
-  ft.m_parsed.m_metadata = true;
+  m_metadata = emo.GetMetadata();
+  m_parsed.m_metadata = true;
 
   CHECK_LESS_OR_EQUAL(emo.GetTypes().Size(), feature::kMaxTypesCount, ());
-  copy(emo.GetTypes().begin(), emo.GetTypes().end(), ft.m_types.begin());
+  copy(emo.GetTypes().begin(), emo.GetTypes().end(), m_types.begin());
 
-  ft.m_parsed.m_types = true;
-  ft.m_header = CalculateHeader(emo.GetTypes().Size(), geomType, ft.m_params);
-  ft.m_parsed.m_header2 = true;
-
-  ft.m_id = emo.GetID();
-  return ft;
-}
-
-void FeatureType::Deserialize(SharedLoadInfo const * loadInfo, Buffer buffer)
-{
-  CHECK(loadInfo, ());
-  m_loadInfo = loadInfo;
-  m_data = buffer;
-  m_header = Header(m_data);
+  m_parsed.m_types = true;
+  m_header = CalculateHeader(emo.GetTypes().Size(), geomType, m_params);
+  m_parsed.m_header2 = true;
 
-  m_offsets.Reset();
-  m_ptsSimpMask = 0;
-  m_limitRect = m2::RectD::GetEmptyRect();
-  m_parsed.Reset();
-  m_innerStats.MakeZero();
+  m_id = emo.GetID();
 }
 
 feature::EGeomType FeatureType::GetFeatureType() const
@@ -322,15 +319,6 @@ int8_t FeatureType::GetLayer()
   return m_params.layer;
 }
 
-void FeatureType::ParseEverything()
-{
-  // Also calls ParseCommon() and ParseTypes().
-  ParseHeader2();
-  ParseGeometry(FeatureType::BEST_GEOMETRY);
-  ParseTriangles(FeatureType::BEST_GEOMETRY);
-  ParseMetadata();
-}
-
 void FeatureType::ParseHeader2()
 {
   if (m_parsed.m_header2)
diff --git a/indexer/feature.hpp b/indexer/feature.hpp
index ed83bec055..15786e4508 100644
--- a/indexer/feature.hpp
+++ b/indexer/feature.hpp
@@ -7,6 +7,7 @@
 #include "geometry/rect2d.hpp"
 
 #include "base/buffer_vector.hpp"
+#include "base/macros.hpp"
 
 #include <array>
 #include <cstdint>
@@ -33,9 +34,8 @@ public:
   using Buffer = char const *;
   using GeometryOffsets = buffer_vector<uint32_t, feature::DataHeader::MAX_SCALES_COUNT>;
 
-  static FeatureType ConstructFromMapObject(osm::MapObject const & emo);
-
-  void Deserialize(feature::SharedLoadInfo const * loadInfo, Buffer buffer);
+  FeatureType(feature::SharedLoadInfo const * loadInfo, Buffer buffer);
+  FeatureType(osm::MapObject const & emo);
 
   feature::EGeomType GetFeatureType() const;
   FeatureParamsBase & GetParams() { return m_params; }
@@ -75,11 +75,6 @@ public:
   void SetID(FeatureID const & id) { m_id = id; }
   FeatureID const & GetID() const { return m_id; }
 
-  /// @name Parse functions.
-  //@{
-  /// Super-method to call all possible Parse* methods.
-  void ParseEverything();
-
   void ResetGeometry();
   uint32_t ParseGeometry(int scale);
   uint32_t ParseTriangles(int scale);
@@ -249,4 +244,6 @@ private:
   uint32_t m_ptsSimpMask = 0;
 
   InnerGeomStat m_innerStats;
+
+  DISALLOW_COPY_AND_MOVE(FeatureType);
 };
diff --git a/indexer/feature_source.cpp b/indexer/feature_source.cpp
index 1231d0541b..83e52f59b2 100644
--- a/indexer/feature_source.cpp
+++ b/indexer/feature_source.cpp
@@ -33,13 +33,13 @@ size_t FeatureSource::GetNumFeatures() const
   return m_vector->GetNumFeatures();
 }
 
-bool FeatureSource::GetOriginalFeature(uint32_t index, FeatureType & feature) const
+unique_ptr<FeatureType> FeatureSource::GetOriginalFeature(uint32_t index) const
 {
   ASSERT(m_handle.IsAlive(), ());
   ASSERT(m_vector != nullptr, ());
-  m_vector->GetByIndex(index, feature);
-  feature.SetID(FeatureID(m_handle.GetId(), index));
-  return true;
+  auto ft = m_vector->GetByIndex(index);
+  ft->SetID(FeatureID(m_handle.GetId(), index));
+  return ft;
 }
 
 FeatureStatus FeatureSource::GetFeatureStatus(uint32_t index) const
@@ -47,10 +47,7 @@ FeatureStatus FeatureSource::GetFeatureStatus(uint32_t index) const
   return FeatureStatus::Untouched;
 }
 
-bool FeatureSource::GetModifiedFeature(uint32_t index, FeatureType & feature) const
-{
-  return false;
-}
+unique_ptr<FeatureType> FeatureSource::GetModifiedFeature(uint32_t index) const { return {}; }
 
 void FeatureSource::ForEachAdditionalFeature(m2::RectD const & rect, int scale,
                                              function<void(uint32_t)> const & fn) const
diff --git a/indexer/feature_source.hpp b/indexer/feature_source.hpp
index e1ce7f628c..b456d2f57a 100644
--- a/indexer/feature_source.hpp
+++ b/indexer/feature_source.hpp
@@ -33,13 +33,13 @@ public:
 
   size_t GetNumFeatures() const;
 
-  bool GetOriginalFeature(uint32_t index, FeatureType & feature) const;
+  std::unique_ptr<FeatureType> GetOriginalFeature(uint32_t index) const;
 
   FeatureID GetFeatureId(uint32_t index) const { return FeatureID(m_handle.GetId(), index); }
 
   virtual FeatureStatus GetFeatureStatus(uint32_t index) const;
 
-  virtual bool GetModifiedFeature(uint32_t index, FeatureType & feature) const;
+  virtual std::unique_ptr<FeatureType> GetModifiedFeature(uint32_t index) const;
 
   // Runs |fn| for each feature, that is not present in the mwm. 
   virtual void ForEachAdditionalFeature(m2::RectD const & rect, int scale,
diff --git a/indexer/features_vector.cpp b/indexer/features_vector.cpp
index 33b7828043..e7f6e96ce5 100644
--- a/indexer/features_vector.cpp
+++ b/indexer/features_vector.cpp
@@ -5,13 +5,12 @@
 #include "platform/constants.hpp"
 #include "platform/mwm_version.hpp"
 
-
-void FeaturesVector::GetByIndex(uint32_t index, FeatureType & ft) const
+std::unique_ptr<FeatureType> FeaturesVector::GetByIndex(uint32_t index) const
 {
   uint32_t offset = 0, size = 0;
   auto const ftOffset = m_table ? m_table->GetFeatureOffset(index) : index;
   m_recordReader.ReadRecord(ftOffset, m_buffer, offset, size);
-  ft.Deserialize(&m_loadInfo, &m_buffer[offset]);
+  return make_unique<FeatureType>(&m_loadInfo, &m_buffer[offset]);
 }
 
 size_t FeaturesVector::GetNumFeatures() const
diff --git a/indexer/features_vector.hpp b/indexer/features_vector.hpp
index 854e752863..14811f0bbe 100644
--- a/indexer/features_vector.hpp
+++ b/indexer/features_vector.hpp
@@ -5,6 +5,7 @@
 
 #include "coding/var_record_reader.hpp"
 
+#include <memory>
 
 namespace feature { class FeaturesOffsetsTable; }
 
@@ -21,7 +22,7 @@ public:
   {
   }
 
-  void GetByIndex(uint32_t index, FeatureType & ft) const;
+  std::unique_ptr<FeatureType> GetByIndex(uint32_t index) const;
 
   size_t GetNumFeatures() const;
 
@@ -29,8 +30,7 @@ public:
   {
     uint32_t index = 0;
     m_recordReader.ForEachRecord([&](uint32_t pos, char const * data, uint32_t /*size*/) {
-      FeatureType ft;
-      ft.Deserialize(&m_loadInfo, data);
+      FeatureType ft(&m_loadInfo, data);
 
       // We can't properly set MwmId here, because FeaturesVector
       // works with FileContainerR, not with MwmId/MwmHandle/MwmValue.
diff --git a/indexer/indexer_tests/editable_map_object_test.cpp b/indexer/indexer_tests/editable_map_object_test.cpp
index 92cdaa07b5..86f1352679 100644
--- a/indexer/indexer_tests/editable_map_object_test.cpp
+++ b/indexer/indexer_tests/editable_map_object_test.cpp
@@ -660,7 +660,7 @@ UNIT_TEST(EditableMapObject_FromFeatureType)
   emo.SetPointType();
   emo.SetMercator(m2::PointD(1.0, 1.0));
 
-  auto ft = FeatureType::ConstructFromMapObject(emo);
+  FeatureType ft(emo);
   EditableMapObject emo2;
   emo2.SetFromFeatureType(ft);
   TEST(emo.GetTypes().Equals(emo2.GetTypes()), ());
diff --git a/indexer/indexer_tests/scale_index_reading_tests.cpp b/indexer/indexer_tests/scale_index_reading_tests.cpp
index 23a73ed3ea..191c8164e8 100644
--- a/indexer/indexer_tests/scale_index_reading_tests.cpp
+++ b/indexer/indexer_tests/scale_index_reading_tests.cpp
@@ -59,11 +59,11 @@ public:
     Names names;
     for (auto const & index : indices)
     {
-      FeatureType ft;
-      TEST(loader.GetFeatureByIndex(index, ft), ("Can't load feature by index:", index));
+      auto ft = loader.GetFeatureByIndex(index);
+      TEST(ft, ("Can't load feature by index:", index));
 
       string name;
-      TEST(ft.GetName(StringUtf8Multilang::kEnglishCode, name),
+      TEST(ft->GetName(StringUtf8Multilang::kEnglishCode, name),
            ("Can't get en name by index:", index));
       names.push_back(name);
     }