From 5135fae37f2895c092a085ee04d16c175bdd6c2d Mon Sep 17 00:00:00 2001 From: Maksim Andrianov Date: Thu, 21 Nov 2019 13:39:18 +0300 Subject: [PATCH] [generator] Check -> IsValid: added more logging. --- .../collector_routing_city_boundaries.cpp | 2 +- generator/feature_builder.cpp | 33 +++++++++++-------- generator/feature_builder.hpp | 3 +- .../generator_tests/feature_builder_test.cpp | 20 +++++------ indexer/feature_data.cpp | 15 +++++---- indexer/feature_data.hpp | 5 ++- 6 files changed, 43 insertions(+), 35 deletions(-) diff --git a/generator/collector_routing_city_boundaries.cpp b/generator/collector_routing_city_boundaries.cpp index fc9e9d02ab..d1b90bbadd 100644 --- a/generator/collector_routing_city_boundaries.cpp +++ b/generator/collector_routing_city_boundaries.cpp @@ -134,7 +134,7 @@ void RoutingCityBoundariesCollector::Collect(OsmElement const & osmElement) while (m_featureMakerSimple.GetNextFeature(feature)) { - if (feature.GetParams().IsValid()) + if (feature.IsValid()) Process(feature, osmElementCopy); } } diff --git a/generator/feature_builder.cpp b/generator/feature_builder.cpp index b42a5ff954..4d4ba40d01 100644 --- a/generator/feature_builder.cpp +++ b/generator/feature_builder.cpp @@ -218,7 +218,7 @@ bool FeatureBuilder::RemoveInvalidTypes() TypesHolder FeatureBuilder::GetTypesHolder() const { - Check(*this); + CHECK(IsValid(), (*this)); TypesHolder holder(m_params.GetGeomType()); for (auto const t : m_params.m_types) @@ -380,7 +380,7 @@ void FeatureBuilder::SerializeBase(Buffer & data, serial::GeometryCodingParams c void FeatureBuilder::SerializeForIntermediate(Buffer & data) const { - Check(*this); + CHECK(IsValid(), (*this)); data.clear(); @@ -471,12 +471,13 @@ void FeatureBuilder::DeserializeFromIntermediate(Buffer & data) rw::ReadVectorOfPOD(source, m_osmIds); - Check(*this); + CHECK(IsValid(), (*this)); } void FeatureBuilder::SerializeAccuratelyForIntermediate(Buffer & data) const { - Check(*this); + CHECK(IsValid(), (*this)); + data.clear(); PushBackByteSink sink(data); m_params.Write(sink, true /* store additional info from FeatureParams */); @@ -531,7 +532,8 @@ void FeatureBuilder::DeserializeAccuratelyFromIntermediate(Buffer & data) } rw::ReadVectorOfPOD(source, m_osmIds); - Check(*this); + + CHECK(IsValid(), (*this)); } void FeatureBuilder::AddOsmId(base::GeoObjectId id) { m_osmIds.push_back(id); } @@ -749,19 +751,24 @@ void FeatureBuilder::SerializeForMwm(SupportingData & data, } } -// Functions -void Check(FeatureBuilder const fb) +bool FeatureBuilder::IsValid() const { - CHECK(fb.GetParams().CheckValid(), (fb)); + if (!GetParams().IsValid()) + return false; - if (fb.IsLine()) - CHECK(fb.GetOuterGeometry().size() >= 2, (fb)); + if (IsLine() && GetOuterGeometry().size() < 2) + return false; - if (fb.IsArea()) + if (IsArea()) { - for (auto const & points : fb.GetGeometry()) - CHECK(points.size() >= 3, (fb)); + for (auto const & points : GetGeometry()) + { + if (points.size() < 3) + return false; + } } + + return true; } string DebugPrint(FeatureBuilder const & fb) diff --git a/generator/feature_builder.hpp b/generator/feature_builder.hpp index cf427d5a9a..41a5c80692 100644 --- a/generator/feature_builder.hpp +++ b/generator/feature_builder.hpp @@ -51,6 +51,8 @@ public: // coordinates is used. bool IsExactEq(FeatureBuilder const & fb) const; + bool IsValid() const; + // To work with geometry. void AddPoint(m2::PointD const & p); void SetHoles(Geometry const & holes); @@ -240,7 +242,6 @@ protected: int64_t m_coastCell; }; -void Check(FeatureBuilder const fb); std::string DebugPrint(FeatureBuilder const & fb); // SerializationPolicy serialization and deserialization. diff --git a/generator/generator_tests/feature_builder_test.cpp b/generator/generator_tests/feature_builder_test.cpp index 1f68c38141..a6a4247513 100644 --- a/generator/generator_tests/feature_builder_test.cpp +++ b/generator/generator_tests/feature_builder_test.cpp @@ -50,7 +50,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_ManyTypes) fb1.SetCenter(m2::PointD(0, 0)); TEST(fb1.RemoveInvalidTypes(), ()); - Check(fb1); + TEST(fb1.IsValid(), (fb1)); FeatureBuilder::Buffer buffer; TEST(fb1.PreSerializeAndRemoveUselessNamesForIntermediate(), ()); @@ -59,7 +59,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_ManyTypes) FeatureBuilder fb2; fb2.DeserializeFromIntermediate(buffer); - Check(fb2); + TEST(fb2.IsValid(), (fb2)); TEST_EQUAL(fb1, fb2, ()); TEST_EQUAL(fb2.GetTypesCount(), 6, ()); } @@ -86,7 +86,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_LineTypes) fb1.SetLinear(); TEST(fb1.RemoveInvalidTypes(), ()); - Check(fb1); + TEST(fb1.IsValid(), (fb1)); FeatureBuilder::Buffer buffer; TEST(fb1.PreSerializeAndRemoveUselessNamesForIntermediate(), ()); @@ -95,7 +95,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_LineTypes) FeatureBuilder fb2; fb2.DeserializeFromIntermediate(buffer); - Check(fb2); + TEST(fb2.IsValid(), (fb2)); TEST_EQUAL(fb1, fb2, ()); TEST_EQUAL(fb2.GetTypesCount(), 5, ()); } @@ -113,7 +113,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_Waterfall) fb1.SetCenter(m2::PointD(1, 1)); TEST(fb1.RemoveInvalidTypes(), ()); - Check(fb1); + TEST(fb1.IsValid(), (fb1)); FeatureBuilder::Buffer buffer; TEST(fb1.PreSerializeAndRemoveUselessNamesForIntermediate(), ()); @@ -122,7 +122,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_Waterfall) FeatureBuilder fb2; fb2.DeserializeFromIntermediate(buffer); - Check(fb2); + TEST(fb2.IsValid(), (fb2)); TEST_EQUAL(fb1, fb2, ()); TEST_EQUAL(fb2.GetTypesCount(), 1, ());; } @@ -196,7 +196,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_RemoveUselessNames) TEST(fb1.GetName(0).empty(), ()); TEST(fb1.GetName(8).empty(), ()); - Check(fb1); + TEST(fb1.IsValid(), (fb1)); } UNIT_CLASS_TEST(TestWithClassificator, FeatureParams_Parsing) @@ -250,7 +250,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FeatureBuilder_SerializeLocalityObjectFor fb.SetCenter(m2::PointD(10.1, 15.8)); TEST(fb.RemoveInvalidTypes(), ()); - Check(fb); + TEST(fb.IsValid(), (fb)); feature::DataHeader header; header.SetGeometryCodingParams(serial::GeometryCodingParams()); @@ -286,7 +286,7 @@ UNIT_TEST(FeatureBuilder_SerializeAccuratelyForIntermediate) fb1.SetLinear(); TEST(fb1.RemoveInvalidTypes(), ()); - Check(fb1); + TEST(fb1.IsValid(), (fb1)); FeatureBuilder::Buffer buffer; TEST(fb1.PreSerializeAndRemoveUselessNamesForIntermediate(), ()); @@ -295,6 +295,6 @@ UNIT_TEST(FeatureBuilder_SerializeAccuratelyForIntermediate) FeatureBuilder fb2; fb2.DeserializeAccuratelyFromIntermediate(buffer); - Check(fb2); + TEST(fb2.IsValid(), (fb2)); TEST(fb1.IsExactEq(fb2), ()); } diff --git a/indexer/feature_data.cpp b/indexer/feature_data.cpp index 59c0605b8b..2d9395778d 100644 --- a/indexer/feature_data.cpp +++ b/indexer/feature_data.cpp @@ -220,9 +220,11 @@ bool FeatureParamsBase::operator == (FeatureParamsBase const & rhs) const layer == rhs.layer && rank == rhs.rank); } -bool FeatureParamsBase::CheckValid() const +bool FeatureParamsBase::IsValid() const { - CHECK(layer > LAYER_LOW && layer < LAYER_HIGH, ()); + if (layer <= LAYER_LOW || layer >= LAYER_HIGH) + return false; + return true; } @@ -397,7 +399,6 @@ void FeatureParams::SetGeomTypePointEx() feature::GeomType FeatureParams::GetGeomType() const { - CheckValid(); switch (m_geomType) { case HeaderGeomType::Line: return GeomType::Line; @@ -408,7 +409,6 @@ feature::GeomType FeatureParams::GetGeomType() const HeaderGeomType FeatureParams::GetHeaderGeomType() const { - CheckValid(); return m_geomType; } @@ -552,11 +552,12 @@ uint32_t FeatureParams::FindType(uint32_t comp, uint8_t level) const return ftype::GetEmptyValue(); } -bool FeatureParams::CheckValid() const +bool FeatureParams::IsValid() const { - CHECK(!m_types.empty() && m_types.size() <= kMaxTypesCount, ()); + if (m_types.empty() || m_types.size() > kMaxTypesCount) + return false; - return FeatureParamsBase::CheckValid(); + return FeatureParamsBase::IsValid(); } uint8_t FeatureParams::GetHeader() const diff --git a/indexer/feature_data.hpp b/indexer/feature_data.hpp index 4d46847b9c..e6764474a7 100644 --- a/indexer/feature_data.hpp +++ b/indexer/feature_data.hpp @@ -145,7 +145,7 @@ struct FeatureParamsBase bool operator == (FeatureParamsBase const & rhs) const; - bool CheckValid() const; + bool IsValid() const; std::string DebugString() const; /// @return true if feature doesn't have any drawable strings (names, houses, etc). @@ -258,7 +258,6 @@ public: m_reverseGeometry = rhs.m_reverseGeometry; } - bool IsValid() const { return !m_types.empty(); } void SetGeomType(feature::GeomType t); void SetGeomTypePointEx(); feature::GeomType GetGeomType() const; @@ -284,7 +283,7 @@ public: /// Find type that matches "comp" with "level" in classificator hierarchy. uint32_t FindType(uint32_t comp, uint8_t level) const; - bool CheckValid() const; + bool IsValid() const; uint8_t GetHeader() const;