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

Author: tatiana-yan

editor/editable_feature_source.cpp

@@ -8,10 +8,10 @@ FeatureStatus EditableFeatureSource::GetFeatureStatus(uint32_t index) const
   return editor.GetFeatureStatus(m_handle.GetId(), index);
 }
 
-bool EditableFeatureSource::GetModifiedFeature(uint32_t index, FeatureType & feature) const
+unique_ptr<FeatureType> EditableFeatureSource::GetModifiedFeature(uint32_t index) const
 {
   osm::Editor & editor = osm::Editor::Instance();
-  return editor.GetEditedFeature(m_handle.GetId(), index, feature);
+  return editor.GetEditedFeature(FeatureID(m_handle.GetId(), index));
 }
 
 void EditableFeatureSource::ForEachAdditionalFeature(m2::RectD const & rect, int scale,

editor/editable_feature_source.hpp

@@ -17,7 +17,7 @@ class EditableFeatureSource final : public FeatureSource
 
   // FeatureSource overrides:
   FeatureStatus GetFeatureStatus(uint32_t index) const override;
-  bool GetModifiedFeature(uint32_t index, FeatureType & feature) const override;
+  std::unique_ptr<FeatureType> GetModifiedFeature(uint32_t index) const override;
   void ForEachAdditionalFeature(m2::RectD const & rect, int scale,
                                 std::function<void(uint32_t)> const & fn) const override;
 };

editor/editor_tests/osm_editor_test.cpp

@@ -249,8 +249,7 @@ void EditorTest::GetEditedFeatureTest()
 
   {
     FeatureID feature;
-    FeatureType ft;
-    TEST(!editor.GetEditedFeature(feature, ft), ());
+    TEST(!editor.GetEditedFeature(feature), ());
   }
 
   auto const mwmId = ConstructTestMwm([](TestMwmBuilder & builder)
@@ -259,21 +258,20 @@ void EditorTest::GetEditedFeatureTest()
     builder.Add(cafe);
   });
 
-  ForEachCafeAtPoint(m_dataSource, m2::PointD(1.0, 1.0), [&editor](FeatureType & ft)
-  {
-    FeatureType featureType;
-    TEST(!editor.GetEditedFeature(ft.GetID(), featureType), ());
+  ForEachCafeAtPoint(m_dataSource, m2::PointD(1.0, 1.0), [&editor](FeatureType & ft) {
+    TEST(!editor.GetEditedFeature(ft.GetID()), ());
 
     SetBuildingLevelsToOne(ft);
 
-    TEST(editor.GetEditedFeature(ft.GetID(), featureType), ());
+    auto featureType = editor.GetEditedFeature(ft.GetID());
+    TEST(featureType, ());
 
     osm::EditableMapObject savedEmo;
-    FillEditableMapObject(editor, featureType, savedEmo);
+    FillEditableMapObject(editor, *featureType, savedEmo);
 
     TEST_EQUAL(savedEmo.GetBuildingLevels(), "1", ());
 
-    TEST_EQUAL(ft.GetID(), featureType.GetID(), ());
+    TEST_EQUAL(ft.GetID(), featureType->GetID(), ());
   });
 }
 
@@ -472,11 +470,10 @@ void EditorTest::DeleteFeatureTest()
   osm::EditableMapObject emo;
   CreateCafeAtPoint({3.0, 3.0}, mwmId, emo);
 
-  FeatureType ft;
-  editor.GetEditedFeature(emo.GetID().m_mwmId, emo.GetID().m_index, ft);
-  editor.DeleteFeature(ft.GetID());
+  auto ft = editor.GetEditedFeature(emo.GetID());
+  editor.DeleteFeature(ft->GetID());
 
-  TEST_EQUAL(editor.GetFeatureStatus(ft.GetID()), FeatureStatus::Untouched, ());
+  TEST_EQUAL(editor.GetFeatureStatus(ft->GetID()), FeatureStatus::Untouched, ());
 
   ForEachCafeAtPoint(m_dataSource, m2::PointD(1.0, 1.0), [&editor](FeatureType & ft)
   {

editor/osm_editor.cpp

@@ -516,21 +516,14 @@ void Editor::ForEachCreatedFeature(MwmSet::MwmId const & id, FeatureIndexFunctor
   }
 }
 
-bool Editor::GetEditedFeature(MwmSet::MwmId const & mwmId, uint32_t index,
-                              FeatureType & outFeature) const
+unique_ptr<FeatureType> Editor::GetEditedFeature(FeatureID const & fid) const
 {
   auto const features = m_features.Get();
-  auto const * featureInfo = GetFeatureTypeInfo(*features, mwmId, index);
+  auto const * featureInfo = GetFeatureTypeInfo(*features, fid.m_mwmId, fid.m_index);
   if (featureInfo == nullptr)
-    return false;
-
-  outFeature = FeatureType::ConstructFromMapObject(featureInfo->m_object);
-  return true;
-}
+    return {};
 
-bool Editor::GetEditedFeature(FeatureID const & fid, FeatureType & outFeature) const
-{
-  return GetEditedFeature(fid.m_mwmId, fid.m_index, outFeature);
+  return make_unique<FeatureType>(featureInfo->m_object);
 }
 
 bool Editor::GetEditedFeatureStreet(FeatureID const & fid, string & outFeatureStreet) const

editor/osm_editor.hpp

@@ -142,10 +142,8 @@ class Editor final : public MwmSet::Observer
   /// Marks feature as "deleted" from MwM file.
   void DeleteFeature(FeatureID const & fid);
 
-  /// @returns false if feature wasn't edited.
-  /// @param outFeature is valid only if true was returned.
-  bool GetEditedFeature(MwmSet::MwmId const & mwmId, uint32_t index, FeatureType & outFeature) const;
-  bool GetEditedFeature(FeatureID const & fid, FeatureType & outFeature) const;
+  /// @returns nullptr if feature wasn't edited.
+  std::unique_ptr<FeatureType> GetEditedFeature(FeatureID const & fid) const;
 
   /// @returns false if feature wasn't edited.
   /// @param outFeatureStreet is valid only if true was returned.

feature_list/feature_list.cpp

@@ -366,9 +366,9 @@ int main(int argc, char ** argv)
     FeaturesLoaderGuard loader(dataSource, mwmId);
     for (uint32_t ftIndex = 0; ftIndex < loader.GetNumFeatures(); ftIndex++)
     {
-      FeatureType ft;
-      if (loader.GetFeatureByIndex(static_cast<uint32_t>(ftIndex), ft))
-        doProcess.Process(ft, featureIdToOsmId);
+      auto ft = loader.GetFeatureByIndex(static_cast<uint32_t>(ftIndex));
+      if (ft)
+        doProcess.Process(*ft, featureIdToOsmId);
     }
     doProcess.ClearCache();
   }

generator/search_index_builder.cpp

@@ -347,12 +347,12 @@ bool GetStreetIndex(search::MwmContext & ctx, uint32_t featureID, string const &
   bool const hasStreet = !streetName.empty();
   if (hasStreet)
   {
-    FeatureType ft;
-    VERIFY(ctx.GetFeature(featureID, ft), ());
+    auto ft = ctx.GetFeature(featureID);
+    CHECK(ft, ());
 
     using TStreet = search::ReverseGeocoder::Street;
     vector<TStreet> streets;
-    search::ReverseGeocoder::GetNearbyStreets(ctx, feature::GetCenter(ft),
+    search::ReverseGeocoder::GetNearbyStreets(ctx, feature::GetCenter(*ft),
                                               true /* includeSquaresAndSuburbs */, streets);
 
     auto const res = search::ReverseGeocoder::GetMatchedStreetIndex(streetName, streets);

indexer/data_source.cpp

@@ -79,24 +79,25 @@ class ReadMWMFunctor
 
 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:
   case FeatureStatus::Obsolete: return;
   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

indexer/data_source.hpp

@@ -100,12 +100,11 @@ class FeaturesLoaderGuard
   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:

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)