From e85177c6853c4ef8c02cfff274e6b64a23157547 Mon Sep 17 00:00:00 2001 From: Maxim Pimenov Date: Tue, 21 Mar 2017 14:33:16 +0300 Subject: [PATCH] Review fixes. --- .../generator_tests/restriction_test.cpp | 15 +++++------ .../generator_tests/road_access_test.cpp | 17 ++++++------ generator/restriction_writer.cpp | 2 +- generator/road_access_generator.cpp | 10 +++++-- generator/road_access_generator.hpp | 4 +-- routing/CMakeLists.txt | 1 + routing/road_access.cpp | 27 +++++++++++++++++++ routing/road_access.hpp | 7 +++-- routing/road_access_serialization.hpp | 5 ++-- routing/routing.pro | 1 + routing/routing_tests/road_access_test.cpp | 4 +-- 11 files changed, 65 insertions(+), 28 deletions(-) create mode 100644 routing/road_access.cpp diff --git a/generator/generator_tests/restriction_test.cpp b/generator/generator_tests/restriction_test.cpp index efed713a51..b02f335f94 100644 --- a/generator/generator_tests/restriction_test.cpp +++ b/generator/generator_tests/restriction_test.cpp @@ -9,14 +9,13 @@ #include "routing/restriction_loader.hpp" -#include "coding/file_container.hpp" -#include "coding/file_name_utils.hpp" - +#include "platform/country_file.hpp" +#include "platform/platform.hpp" #include "platform/platform_tests_support/scoped_dir.hpp" #include "platform/platform_tests_support/scoped_file.hpp" -#include "platform/country_file.hpp" -#include "platform/platform.hpp" +#include "coding/file_container.hpp" +#include "coding/file_name_utils.hpp" #include "base/logging.hpp" @@ -61,7 +60,7 @@ void LoadRestrictions(string const & mwmFilePath, RestrictionVec & restrictions) } catch (Reader::OpenException const & e) { - LOG(LERROR, ("Error while reading", RESTRICTIONS_FILE_TAG, "section.", e.Msg())); + TEST(false, ("Error while reading", ROAD_ACCESS_FILE_TAG, "section.", e.Msg())); } } @@ -88,8 +87,8 @@ void TestRestrictionBuilding(string const & restrictionContent, string const & m // Creating osm ids to feature ids mapping. string const mappingRelativePath = my::JoinPath(kTestDir, kOsmIdsToFeatureIdsName); - ScopedFile const mappingScopedFile(mappingRelativePath); - string const mappingFullPath = my::JoinPath(writableDir, mappingRelativePath); + ScopedFile const mappingFile(mappingRelativePath); + string const mappingFullPath = mappingFile.GetFullPath(); ReEncodeOsmIdsToFeatureIdsMapping(mappingContent, mappingFullPath); // Adding restriction section to mwm. diff --git a/generator/generator_tests/road_access_test.cpp b/generator/generator_tests/road_access_test.cpp index f09a2765f9..58a9ef0199 100644 --- a/generator/generator_tests/road_access_test.cpp +++ b/generator/generator_tests/road_access_test.cpp @@ -8,14 +8,13 @@ #include "routing/road_access_serialization.hpp" -#include "coding/file_container.hpp" -#include "coding/file_name_utils.hpp" - +#include "platform/country_file.hpp" +#include "platform/platform.hpp" #include "platform/platform_tests_support/scoped_dir.hpp" #include "platform/platform_tests_support/scoped_file.hpp" -#include "platform/country_file.hpp" -#include "platform/platform.hpp" +#include "coding/file_container.hpp" +#include "coding/file_name_utils.hpp" #include "base/logging.hpp" @@ -54,7 +53,7 @@ void LoadRoadAccess(string const & mwmFilePath, RoadAccess & roadAccess) } catch (Reader::OpenException const & e) { - LOG(LERROR, ("Error while reading", ROAD_ACCESS_FILE_TAG, "section.", e.Msg())); + TEST(false, ("Error while reading", ROAD_ACCESS_FILE_TAG, "section.", e.Msg())); } } @@ -78,8 +77,8 @@ void TestRoadAccess(string const & roadAccessContent, string const & mappingCont // Creating osm ids to feature ids mapping. string const mappingRelativePath = my::JoinPath(kTestDir, kOsmIdsToFeatureIdsName); - ScopedFile const mappingScopedFile(mappingRelativePath); - string const mappingFullPath = my::JoinPath(writableDir, mappingRelativePath); + ScopedFile const mappingFile(mappingRelativePath); + string const mappingFullPath = mappingFile.GetFullPath(); ReEncodeOsmIdsToFeatureIdsMapping(mappingContent, mappingFullPath); // Adding road access section to mwm. @@ -92,7 +91,7 @@ void TestRoadAccess(string const & roadAccessContent, string const & mappingCont LoadRoadAccess(mwmFullPath, roadAccessFromMwm); RoadAccessCollector const collector(roadAccessFullPath, mappingFullPath); TEST(collector.IsValid(), ()); - TEST(roadAccessFromMwm == collector.GetRoadAccess(), ()); + TEST_EQUAL(roadAccessFromMwm, collector.GetRoadAccess(), ()); } UNIT_TEST(RoadAccess_Smoke) diff --git a/generator/restriction_writer.cpp b/generator/restriction_writer.cpp index 52d405f513..27bba5e508 100644 --- a/generator/restriction_writer.cpp +++ b/generator/restriction_writer.cpp @@ -98,5 +98,5 @@ void RestrictionWriter::Write(RelationElement const & relationElement) m_stream << ToString(type) << "," << fromIt->first << ", " << toIt->first << '\n'; } -bool RestrictionWriter::IsOpened() const { return m_stream.is_open() && !m_stream.fail(); } +bool RestrictionWriter::IsOpened() const { return m_stream && m_stream.is_open(); } } // namespace routing diff --git a/generator/road_access_generator.cpp b/generator/road_access_generator.cpp index 6223826655..e16dca38db 100644 --- a/generator/road_access_generator.cpp +++ b/generator/road_access_generator.cpp @@ -24,6 +24,7 @@ #include #include #include +#include using namespace std; @@ -44,6 +45,8 @@ bool ParseRoadAccess(string const & roadAccessPath, return false; } + vector privateRoads; + string line; for (uint32_t lineNo = 1;; ++lineNo) { @@ -76,9 +79,11 @@ bool ParseRoadAccess(string const & roadAccessPath, } uint32_t const featureId = it->second; - roadAccess.GetPrivateRoads().emplace_back(featureId); + privateRoads.emplace_back(featureId); } + roadAccess.SetPrivateRoads(move(privateRoads)); + return true; } } // namespace @@ -122,7 +127,8 @@ void RoadAccessWriter::Process(OsmElement const & elem, FeatureParams const & pa m_stream << kBarrierGate << " " << elem.id << endl; } -bool RoadAccessWriter::IsOpened() const { return m_stream.is_open() && !m_stream.fail(); } +bool RoadAccessWriter::IsOpened() const { return m_stream && m_stream.is_open(); } + // RoadAccessCollector ---------------------------------------------------------- RoadAccessCollector::RoadAccessCollector(string const & roadAccessPath, string const & osmIdsToFeatureIdsPath) diff --git a/generator/road_access_generator.hpp b/generator/road_access_generator.hpp index 75f750237c..e0726c56c2 100644 --- a/generator/road_access_generator.hpp +++ b/generator/road_access_generator.hpp @@ -1,9 +1,9 @@ #pragma once -#include "routing/road_access.hpp" - #include "generator/intermediate_elements.hpp" +#include "routing/road_access.hpp" + #include #include #include diff --git a/routing/CMakeLists.txt b/routing/CMakeLists.txt index 143fa4d2f0..1e07dd43a0 100644 --- a/routing/CMakeLists.txt +++ b/routing/CMakeLists.txt @@ -77,6 +77,7 @@ set( restriction_loader.hpp restrictions_serialization.cpp restrictions_serialization.hpp + road_access.cpp road_access.hpp road_access_serialization.cpp road_access_serialization.hpp diff --git a/routing/road_access.cpp b/routing/road_access.cpp new file mode 100644 index 0000000000..b4adc915a4 --- /dev/null +++ b/routing/road_access.cpp @@ -0,0 +1,27 @@ +#include "routing/road_access.hpp" + +#include + +namespace routing +{ +std::string DebugPrint(RoadAccess const & r) +{ + size_t const kMaxIdsToShow = 10; + std::ostringstream oss; + oss << "RoadAccess: Private roads ["; + auto const & privateRoads = r.GetPrivateRoads(); + for (size_t i = 0; i < privateRoads.size(); ++i) + { + if (i == kMaxIdsToShow) + { + oss << "..."; + break; + } + if (i > 0) + oss << " "; + oss << privateRoads[i]; + } + oss << "]"; + return oss.str(); +} +} // namespace routing diff --git a/routing/road_access.hpp b/routing/road_access.hpp index eff4d7fdc1..d1a10ca071 100644 --- a/routing/road_access.hpp +++ b/routing/road_access.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include namespace routing @@ -11,10 +12,10 @@ namespace routing class RoadAccess final { public: - std::vector & GetPrivateRoads() { return m_privateRoads; } std::vector const & GetPrivateRoads() const { return m_privateRoads; } - void StealPrivateRoads(std::vector && v) { m_privateRoads = std::move(v); } + template + void SetPrivateRoads(V && v) { m_privateRoads = std::forward(v); } void Clear() { m_privateRoads.clear(); } @@ -26,4 +27,6 @@ private: // Feature ids of blocked features in the corresponding mwm. std::vector m_privateRoads; }; + +std::string DebugPrint(RoadAccess const & r); } // namespace routing diff --git a/routing/road_access_serialization.hpp b/routing/road_access_serialization.hpp index 483d1b1954..8ae3f69a69 100644 --- a/routing/road_access_serialization.hpp +++ b/routing/road_access_serialization.hpp @@ -11,6 +11,7 @@ #include #include +#include namespace routing { @@ -43,8 +44,7 @@ public: uint32_t const header = ReadPrimitiveFromSource(src); CHECK_EQUAL(header, kLatestVersion, ()); size_t numPrivateRoads = base::checked_cast(ReadPrimitiveFromSource(src)); - auto & privateRoads = roadAccess.GetPrivateRoads(); - privateRoads.resize(numPrivateRoads); + std::vector privateRoads(numPrivateRoads); if (numPrivateRoads > 0) privateRoads[0] = ReadVarUint(src); for (size_t i = 1; i < numPrivateRoads; ++i) @@ -52,6 +52,7 @@ public: uint32_t delta = ReadVarUint(src); privateRoads[i] = privateRoads[i - 1] + delta; } + roadAccess.SetPrivateRoads(move(privateRoads)); } private: diff --git a/routing/routing.pro b/routing/routing.pro index fb6490b4ab..ed0992c853 100644 --- a/routing/routing.pro +++ b/routing/routing.pro @@ -43,6 +43,7 @@ SOURCES += \ osrm_helpers.cpp \ osrm_path_segment_factory.cpp \ pedestrian_directions.cpp \ + road_access.cpp \ road_access_serialization.cpp \ restriction_loader.cpp \ restrictions_serialization.cpp \ diff --git a/routing/routing_tests/road_access_test.cpp b/routing/routing_tests/road_access_test.cpp index 0f08eb855f..5554eff638 100644 --- a/routing/routing_tests/road_access_test.cpp +++ b/routing/routing_tests/road_access_test.cpp @@ -19,7 +19,7 @@ UNIT_TEST(RoadAccess_Serialization) { vector privateRoads = {1, 2, 3, 100}; RoadAccess roadAccess; - roadAccess.StealPrivateRoads(move(privateRoads)); + roadAccess.SetPrivateRoads(move(privateRoads)); vector buf; { @@ -34,7 +34,7 @@ UNIT_TEST(RoadAccess_Serialization) RoadAccessSerializer::Deserialize(src, deserializedRoadAccess); } - TEST_EQUAL(roadAccess.GetPrivateRoads(), deserializedRoadAccess.GetPrivateRoads(), ()); + TEST_EQUAL(roadAccess, deserializedRoadAccess, ()); { auto const & b = deserializedRoadAccess.GetPrivateRoads();