From 527000b42016aeb93e70a2508187ac59c61ceaa9 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Thu, 10 Nov 2016 18:46:03 +0300 Subject: [PATCH] Review fixes. --- .../restriction_collector_test.cpp | 2 +- generator/osm_source.cpp | 8 ++--- generator/restriction_collector.cpp | 33 +++++++++++++------ generator/restriction_collector.hpp | 2 +- generator/restriction_writer.cpp | 8 ++--- routing/restriction_loader.cpp | 2 +- routing/restriction_loader.hpp | 2 +- routing/routing.pro | 4 +-- ...rializer.cpp => routing_serialization.cpp} | 2 +- ...rializer.hpp => routing_serialization.hpp} | 6 ++-- 10 files changed, 41 insertions(+), 28 deletions(-) rename routing/{routing_serializer.cpp => routing_serialization.cpp} (96%) rename routing/{routing_serializer.hpp => routing_serialization.hpp} (98%) diff --git a/generator/generator_tests/restriction_collector_test.cpp b/generator/generator_tests/restriction_collector_test.cpp index aa886f7a2d..233e8f952f 100644 --- a/generator/generator_tests/restriction_collector_test.cpp +++ b/generator/generator_tests/restriction_collector_test.cpp @@ -3,7 +3,7 @@ #include "generator/osm_id.hpp" #include "generator/restriction_collector.hpp" -#include "routing/routing_serializer.hpp" +#include "routing/routing_serialization.hpp" #include "platform/platform_tests_support/scoped_dir.hpp" #include "platform/platform_tests_support/scoped_file.hpp" diff --git a/generator/osm_source.cpp b/generator/osm_source.cpp index 40d3d7fd1f..4e6695540e 100644 --- a/generator/osm_source.cpp +++ b/generator/osm_source.cpp @@ -337,13 +337,13 @@ public: m_coastsHolder.reset( new feature::FeaturesCollector(info.GetTmpFileName(WORLD_COASTS_FILE_NAME))); - string const featureId2OsmIdsFile = info.GetIntermediateFileName(info.m_featureIdToOsmIds, ""); - LOG(LINFO, ("Saving mapping from feature ids to osm ids to", featureId2OsmIdsFile)); - m_featureIdToOsmIds.Open(featureId2OsmIdsFile); + string const featureIdToOsmIdsFile = info.GetIntermediateFileName(info.m_featureIdToOsmIds, ""); + LOG(LINFO, ("Saving mapping from feature ids to osm ids to", featureIdToOsmIdsFile)); + m_featureIdToOsmIds.Open(featureIdToOsmIdsFile); if (!m_featureIdToOsmIds.IsOpened()) { LOG(LWARNING, - ("Cannot open", featureId2OsmIdsFile, ". Feature ids to osm ids to map won't be saved.")); + ("Cannot open", featureIdToOsmIdsFile, ". Feature ids to osm ids to map won't be saved.")); } if (info.m_splitByPolygons || !info.m_fileName.empty()) diff --git a/generator/restriction_collector.cpp b/generator/restriction_collector.cpp index 33bc95dc9e..ba0395f25a 100644 --- a/generator/restriction_collector.cpp +++ b/generator/restriction_collector.cpp @@ -33,18 +33,24 @@ namespace routing RestrictionCollector::RestrictionCollector(string const & restrictionPath, string const & featureIdToOsmIdsPath) { - MY_SCOPE_GUARD(parseErrorGuard, [this]{ + MY_SCOPE_GUARD(clean, [this](){ m_osmIdToFeatureId.clear(); m_restrictions.clear(); }); - if (!ParseFeatureId2OsmIdsMapping(featureIdToOsmIdsPath) - || !ParseRestrictions(restrictionPath)) + if (!ParseFeatureId2OsmIdsMapping(featureIdToOsmIdsPath)) { - LOG(LWARNING, ("An error happened while parsing", featureIdToOsmIdsPath, "or", restrictionPath)); + LOG(LWARNING, ("An error happened while parsing feature id to osm ids mapping from file:", + featureIdToOsmIdsPath)); return; } - parseErrorGuard.release(); + + if (!ParseRestrictions(restrictionPath)) + { + LOG(LWARNING, ("An error happened while parsing restrictions from file:", restrictionPath)); + return; + } + clean.release(); my::SortUnique(m_restrictions); @@ -61,20 +67,27 @@ bool RestrictionCollector::IsValid() const bool RestrictionCollector::ParseFeatureId2OsmIdsMapping(string const & featureIdToOsmIdsPath) { - ifstream featureId2OsmIdsStream(featureIdToOsmIdsPath); - if (featureId2OsmIdsStream.fail()) + ifstream featureIdToOsmIdsStream(featureIdToOsmIdsPath); + if (featureIdToOsmIdsStream.fail()) return false; string line; - while (getline(featureId2OsmIdsStream, line)) + while (getline(featureIdToOsmIdsStream, line)) { vector osmIds; strings::SimpleTokenizer iter(line, kDelim); if (!ParseLineOfNumbers(iter, osmIds)) + { + LOG(LWARNING, ("Cannot parse feature id to osm ids mapping. Line:", line)); return false; + } if (osmIds.size() < 2) + { + LOG(LWARNING, ("Parse result of mapping feature id to osm ids is too small. Line:", + line)); return false; // Every line should contain feature id and at least one osm id. + } uint32_t const featureId = static_cast(osmIds.front()); osmIds.erase(osmIds.begin()); @@ -99,7 +112,7 @@ bool RestrictionCollector::ParseRestrictions(string const & path) Restriction::Type type; if (!FromString(*iter, type)) { - LOG(LWARNING, ("Cannot parse a restriction type", *iter, "form", path)); + LOG(LWARNING, ("Cannot parse a restriction type. Line:", line)); return false; } @@ -140,7 +153,7 @@ bool RestrictionCollector::AddRestriction(Restriction::Type type, vector const & osmIds) { // Note. One |featureId| could correspond to several osm ids. - // but for road feature |featureId| corresponds exactly one osm id. + // But for road feature |featureId| corresponds to exactly one osm id. for (uint64_t const & osmId : osmIds) { auto const result = m_osmIdToFeatureId.insert(make_pair(osmId, featureId)); diff --git a/generator/restriction_collector.hpp b/generator/restriction_collector.hpp index 8f6e60ae8d..19a04a1f95 100644 --- a/generator/restriction_collector.hpp +++ b/generator/restriction_collector.hpp @@ -1,6 +1,6 @@ #pragma once -#include "routing/routing_serializer.hpp" +#include "routing/routing_serialization.hpp" #include "std/functional.hpp" #include "std/limits.hpp" diff --git a/generator/restriction_writer.cpp b/generator/restriction_writer.cpp index f8550f2549..b2dbea221f 100644 --- a/generator/restriction_writer.cpp +++ b/generator/restriction_writer.cpp @@ -4,7 +4,7 @@ #include "generator/osm_id.hpp" #include "generator/restriction_collector.hpp" -#include "routing/routing_serializer.hpp" +#include "routing/routing_serialization.hpp" #include "base/logging.hpp" @@ -92,13 +92,13 @@ void RestrictionWriter::Write(RelationElement const & relationElement) // Extracting type of restriction. auto const tagIt = relationElement.tags.find("restriction"); if (tagIt == relationElement.tags.end()) - return; // Type of the element is different from "restriction". + return; Restriction::Type type = Restriction::Type::No; if (!TagToType(tagIt->second, type)) - return; // Unsupported restriction type. + return; // Adding restriction. - m_stream << ToString(type) << "," << fromIt->first << ", " << toIt->first << endl; + m_stream << ToString(type) << "," << fromIt->first << ", " << toIt->first << '\n'; } } // namespace routing diff --git a/routing/restriction_loader.cpp b/routing/restriction_loader.cpp index afb032a3a9..2ee1d349bf 100644 --- a/routing/restriction_loader.cpp +++ b/routing/restriction_loader.cpp @@ -1,5 +1,5 @@ #include "routing/restriction_loader.hpp" -#include "routing/routing_serializer.hpp" +#include "routing/routing_serialization.hpp" namespace routing { diff --git a/routing/restriction_loader.hpp b/routing/restriction_loader.hpp index 999e9021b1..5cb2a8987a 100644 --- a/routing/restriction_loader.hpp +++ b/routing/restriction_loader.hpp @@ -1,6 +1,6 @@ #pragma once -#include "routing/routing_serializer.hpp" +#include "routing/routing_serialization.hpp" #include "indexer/index.hpp" diff --git a/routing/routing.pro b/routing/routing.pro index 6a82def091..e4792d64ec 100644 --- a/routing/routing.pro +++ b/routing/routing.pro @@ -41,7 +41,7 @@ SOURCES += \ router_delegate.cpp \ routing_algorithm.cpp \ routing_mapping.cpp \ - routing_serializer.cpp \ + routing_serialization.cpp \ routing_session.cpp \ speed_camera.cpp \ turns.cpp \ @@ -86,7 +86,7 @@ HEADERS += \ routing_helpers.hpp \ routing_mapping.hpp \ routing_result_graph.hpp \ - routing_serializer.hpp \ + routing_serialization.hpp \ routing_session.hpp \ routing_settings.hpp \ speed_camera.hpp \ diff --git a/routing/routing_serializer.cpp b/routing/routing_serialization.cpp similarity index 96% rename from routing/routing_serializer.cpp rename to routing/routing_serialization.cpp index 209e94e96e..d1e5a3a366 100644 --- a/routing/routing_serializer.cpp +++ b/routing/routing_serialization.cpp @@ -1,4 +1,4 @@ -#include "routing/routing_serializer.hpp" +#include "routing/routing_serialization.hpp" namespace { diff --git a/routing/routing_serializer.hpp b/routing/routing_serialization.hpp similarity index 98% rename from routing/routing_serializer.hpp rename to routing/routing_serialization.hpp index e9355c1493..87a7782a7b 100644 --- a/routing/routing_serializer.hpp +++ b/routing/routing_serialization.hpp @@ -131,6 +131,7 @@ private: routing::Restriction::Type const type = begin->m_type; uint32_t prevFirstLinkFeatureId = 0; + BitWriter bits(sink); for (; begin != end; ++begin) { CHECK_EQUAL(type, begin->m_type, ()); @@ -139,7 +140,6 @@ private: CHECK(restriction.IsValid(), ()); CHECK_LESS(1, restriction.m_featureIds.size(), ("No sense in zero or one link restrictions.")); - BitWriter bits(sink); coding::DeltaCoder::Encode(bits, restriction.m_featureIds.size() - 1 /* number of link is two or more */); CHECK_LESS_OR_EQUAL(prevFirstLinkFeatureId, restriction.m_featureIds[0], ()); @@ -160,9 +160,9 @@ private: routing::RestrictionVec & restrictions, Source & src) { uint32_t prevFirstLinkFeatureId = 0; + BitReader bits(src); for (size_t i = 0; i < count; ++i) { - BitReader bits(src); uint32_t const biasedLinkNumber = coding::DeltaCoder::Decode(bits); if (biasedLinkNumber == 0) { @@ -176,7 +176,7 @@ private: uint32_t const biasedFirstFeatureId = coding::DeltaCoder::Decode(bits); if (biasedFirstFeatureId == 0) { - LOG(LERROR, ("Decoded first link restriction feature id delta is zero..")); + LOG(LERROR, ("Decoded first link restriction feature id delta is zero.")); return false; } restriction.m_featureIds[0] = prevFirstLinkFeatureId + biasedFirstFeatureId - 1;