From d9eb999908d2af7f479263a926ecb09c54cd2f51 Mon Sep 17 00:00:00 2001 From: gmoryes Date: Fri, 14 Feb 2020 13:41:22 +0300 Subject: [PATCH] [routing] review fixes --- routing/road_access_serialization.hpp | 53 ++++++++++++---------- routing/routing_tests/road_access_test.cpp | 2 - 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/routing/road_access_serialization.hpp b/routing/road_access_serialization.hpp index 6f3e863e7f..93fa37668f 100644 --- a/routing/road_access_serialization.hpp +++ b/routing/road_access_serialization.hpp @@ -30,8 +30,8 @@ namespace routing class RoadAccessSerializer final { public: - using RoadAccessTypesFeatureMap = ska::flat_hash_map; - using RoadAccessTypesPointMap = ska::flat_hash_map; + using WayToAccess = RoadAccess::WayToAccess; + using PointToAccess = RoadAccess::PointToAccess; using RoadAccessByVehicleType = std::array(VehicleType::Count)>; RoadAccessSerializer() = delete; @@ -55,19 +55,16 @@ public: private: inline static uint32_t const kLatestVersion = 1; - inline static std::map const kSupportedVehicles = { - {VehicleType::Pedestrian, true}, - {VehicleType::Bicycle, true}, - {VehicleType::Car, true}, - {VehicleType::Transit, false}}; - template static void SerializeAccess(Sink & sink, RoadAccessByVehicleType const & roadAccessByType) { auto const sectionSizesPos = sink.Pos(); - std::array(VehicleType::Count)> sectionSizes = {}; + std::array(VehicleType::Count)> sectionSizes; for (size_t i = 0; i < sectionSizes.size(); ++i) - WriteToSink(sink, 0); + { + sectionSizes[i] = 0; + WriteToSink(sink, sectionSizes[i]); + } for (size_t i = 0; i < static_cast(VehicleType::Count); ++i) { @@ -89,39 +86,45 @@ private: static void DeserializeAccess(Source & src, VehicleType vehicleType, RoadAccess & roadAccess) { std::array(VehicleType::Count)> sectionSizes{}; + static_assert(static_cast(VehicleType::Count) == 4, + "It is assumed below that there are only 4 vehicle types and we store 4 numbers " + "of sections size. If you add or remove vehicle type you should up " + "|kLatestVersion| and save back compatibility here."); + for (auto & sectionSize : sectionSizes) sectionSize = ReadPrimitiveFromSource(src); for (size_t i = 0; i < sectionSizes.size(); ++i) { auto const sectionVehicleType = static_cast(i); - if (!kSupportedVehicles.at(vehicleType) || sectionVehicleType != vehicleType) + if (sectionVehicleType != vehicleType) { src.Skip(sectionSizes[i]); continue; } - RoadAccessTypesFeatureMap mf; - RoadAccessTypesPointMap mp; - DeserializeOneVehicleType(src, mf, mp); + WayToAccess wayToAccess; + PointToAccess pointToAccess; + DeserializeOneVehicleType(src, wayToAccess, pointToAccess); - roadAccess.SetAccess(std::move(mf), std::move(mp)); + roadAccess.SetAccess(std::move(wayToAccess), std::move(pointToAccess)); + return; } } template - static void SerializeOneVehicleType(Sink & sink, RoadAccessTypesFeatureMap const & mf, - RoadAccessTypesPointMap const & mp) + static void SerializeOneVehicleType(Sink & sink, WayToAccess const & wayToAccess, + PointToAccess const & pointToAccess) { std::array, static_cast(RoadAccess::Type::Count)> segmentsByRoadAccessType; - for (auto const & kv : mf) + for (auto const & kv : wayToAccess) { segmentsByRoadAccessType[static_cast(kv.second)].push_back( Segment(kFakeNumMwmId, kv.first, 0 /* wildcard segmentIdx */, true /* widcard forward */)); } // For nodes we store |pointId + 1| because 0 is reserved for wildcard segmentIdx. - for (auto const & kv : mp) + for (auto const & kv : pointToAccess) { segmentsByRoadAccessType[static_cast(kv.second)].push_back( Segment(kFakeNumMwmId, kv.first.GetFeatureId(), kv.first.GetPointId() + 1, true)); @@ -135,11 +138,11 @@ private: } template - static void DeserializeOneVehicleType(Source & src, RoadAccessTypesFeatureMap & mf, - RoadAccessTypesPointMap & mp) + static void DeserializeOneVehicleType(Source & src, WayToAccess & wayToAccess, + PointToAccess & pointToAccess) { - mf.clear(); - mp.clear(); + wayToAccess.clear(); + pointToAccess.clear(); for (size_t i = 0; i < static_cast(RoadAccess::Type::Count); ++i) { // An earlier version allowed blocking any segment of a feature (or the entire feature @@ -154,12 +157,12 @@ private: if (seg.GetSegmentIdx() == 0) { // Wildcard segmentIdx. - mf[seg.GetFeatureId()] = static_cast(i); + wayToAccess[seg.GetFeatureId()] = static_cast(i); } else { // For nodes we store |pointId + 1| because 0 is reserved for wildcard segmentIdx. - mp[RoadPoint(seg.GetFeatureId(), seg.GetSegmentIdx() - 1)] = + pointToAccess[RoadPoint(seg.GetFeatureId(), seg.GetSegmentIdx() - 1)] = static_cast(i); } } diff --git a/routing/routing_tests/road_access_test.cpp b/routing/routing_tests/road_access_test.cpp index 31fb2fff82..df7950c642 100644 --- a/routing/routing_tests/road_access_test.cpp +++ b/routing/routing_tests/road_access_test.cpp @@ -54,7 +54,6 @@ UNIT_TEST(RoadAccess_Serialization) MemReader memReader(buf.data(), buf.size()); ReaderSource src(memReader); RoadAccessSerializer::Deserialize(src, VehicleType::Car, deserializedRoadAccess); - TEST_EQUAL(src.Size(), 0, ()); TEST_EQUAL(roadAccessCar, deserializedRoadAccess, ()); } @@ -65,7 +64,6 @@ UNIT_TEST(RoadAccess_Serialization) MemReader memReader(buf.data(), buf.size()); ReaderSource src(memReader); RoadAccessSerializer::Deserialize(src, VehicleType::Pedestrian, deserializedRoadAccess); - TEST_EQUAL(src.Size(), 0, ()); TEST_EQUAL(roadAccessPedestrian, deserializedRoadAccess, ()); }