From b2b36cf0be53a467ec21e690dd56d577c8fd2338 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 18 Oct 2017 10:50:15 +0300 Subject: [PATCH] Review fixes. --- generator/transit_generator.cpp | 30 +++++++++++-------------- routing_common/CMakeLists.txt | 2 +- routing_common/routing_common.pro | 2 +- routing_common/transit_speed_limits.hpp | 8 +++---- routing_common/transit_types.cpp | 12 +++++++--- routing_common/transit_types.hpp | 2 ++ 6 files changed, 30 insertions(+), 26 deletions(-) diff --git a/generator/transit_generator.cpp b/generator/transit_generator.cpp index 2e3f352b16..ada7975549 100644 --- a/generator/transit_generator.cpp +++ b/generator/transit_generator.cpp @@ -48,7 +48,7 @@ using namespace std; namespace { -bool CompStopsById(Stop const & s1, Stop const & s2) +bool LessById(Stop const & s1, Stop const & s2) { return s1.GetId() < s2.GetId(); } @@ -56,12 +56,13 @@ bool CompStopsById(Stop const & s1, Stop const & s2) /// \returns ref to a stop at |stops| by |stopId|. Stop const & FindStopById(vector const & stops, StopId stopId) { + ASSERT(is_sorted(stops.cbegin(), stops.cend(), LessById), ()); auto s1Id = equal_range(stops.cbegin(), stops.cend(), - Stop(stopId, kInvalidFeatureId, kInvalidTransferId, {}, m2::PointD(), {}), - CompStopsById); + Stop(stopId, kInvalidFeatureId, kInvalidTransferId, {}, m2::PointD(), {}), + LessById); CHECK(s1Id.first != stops.cend(), ("No a stop with id:", stopId, "in stops:", stops)); CHECK_EQUAL(distance(s1Id.first, s1Id.second), 1, ("A stop with id:", stopId, "is not unique in stops:", stops)); - return *(s1Id.first); + return *s1Id.first; } /// Extracts the file name from |filePath| and drops all extensions. @@ -148,12 +149,7 @@ void DeserializeGatesFromJson(my::Json const & root, string const & mwmDir, stri template bool IsValid(vector const & items) { - for (auto const & i : items) - { - if (!i.IsValid()) - return false; - } - return true; + return all_of(items.cbegin(), items.cend(), [](Item const & item) { return item.IsValid(); }); } /// \brief Reads from |root| (json) and serializes an array to |serializer|. @@ -166,21 +162,21 @@ void SerializeObject(my::Json const & root, string const & key, Serializer const & stops, vector & edges) { - CHECK(is_sorted(stops.cbegin(), stops.cend(), CompStopsById), ()); + CHECK(is_sorted(stops.cbegin(), stops.cend(), LessById), ()); for (auto & e : edges) { + if (e.GetWeight() != kInvalidWeight) + continue; + Stop const & s1 = FindStopById(stops, e.GetStop1Id()); Stop const & s2 = FindStopById(stops, e.GetStop2Id()); double const lengthInMeters = MercatorBounds::DistanceOnEarth(s1.GetPoint(), s2.GetPoint()); - // Note. 3.6 == 3600 (number of seconds in an hour) / 1000 (number of meters in a kilometer). - // In other words: weighInSec = 3600 * lengthInMeters / (1000 * kTransitAverageSpeedKMpH); - double const weighInSec = 3.6 * lengthInMeters / kTransitAverageSpeedKMpH; - e.SetWeight(weighInSec); + e.SetWeight(lengthInMeters / kTransitAverageSpeedMPS); } } } // namespace @@ -263,7 +259,7 @@ void BuildTransit(string const & mwmPath, string const & transitDir) vector stops; DeserializeFromJson(root, "stops", stops); CHECK(IsValid(stops), ("stops:", stops)); - sort(stops.begin(), stops.end(), CompStopsById); + sort(stops.begin(), stops.end(), LessById); serializer(stops); header.m_gatesOffset = base::checked_cast(w.Pos() - startOffset); diff --git a/routing_common/CMakeLists.txt b/routing_common/CMakeLists.txt index 796fec323b..6a7f7f53e4 100644 --- a/routing_common/CMakeLists.txt +++ b/routing_common/CMakeLists.txt @@ -9,8 +9,8 @@ set( num_mwm_id.hpp pedestrian_model.cpp pedestrian_model.hpp - transit_speed_limits.hpp transit_serdes.hpp + transit_speed_limits.hpp transit_types.cpp transit_types.hpp vehicle_model.cpp diff --git a/routing_common/routing_common.pro b/routing_common/routing_common.pro index a303607dc8..a8cfd46106 100644 --- a/routing_common/routing_common.pro +++ b/routing_common/routing_common.pro @@ -25,7 +25,7 @@ HEADERS += \ car_model.hpp \ num_mwm_id.hpp \ pedestrian_model.hpp \ - transit_speed_limits.hpp \ transit_serdes.hpp \ + transit_speed_limits.hpp \ transit_types.hpp \ vehicle_model.hpp \ diff --git a/routing_common/transit_speed_limits.hpp b/routing_common/transit_speed_limits.hpp index fa3241c487..9338668b6c 100644 --- a/routing_common/transit_speed_limits.hpp +++ b/routing_common/transit_speed_limits.hpp @@ -5,9 +5,9 @@ namespace routing namespace transit { double constexpr kTransitMaxSpeedKMpH = 400.0; -// @TODO(bykoianko, Zverik) Edge and gate weights should be valid always valid. This weights should come -// from transit graph json. But now it'not so. |kTransitAverageSpeedKMpH| should be used now only for -// weight calculating weight at transit section generation stage and should be removed later. -double constexpr kTransitAverageSpeedKMpH = 40.0; +// @TODO(bykoianko, Zverik) Edge and gate weights should be always valid. This weights should come +// from transit graph json. But now it's not so. |kTransitAverageSpeedMPS| should be used now only for +// weight calculating at transit section generation stage and the constant should be removed later. +double constexpr kTransitAverageSpeedMPS = 11.0; } // namespace transit } // namespace routing diff --git a/routing_common/transit_types.cpp b/routing_common/transit_types.cpp index ee2515f0cf..169d3fc1bc 100644 --- a/routing_common/transit_types.cpp +++ b/routing_common/transit_types.cpp @@ -119,7 +119,7 @@ bool SingleMwmSegment::IsEqualForTesting(SingleMwmSegment const & s) const bool SingleMwmSegment::IsValid() const { - return true; + return m_featureId != kInvalidFeatureId; } // Gate ------------------------------------------------------------------------------------------- @@ -145,7 +145,7 @@ bool Gate::IsEqualForTesting(Gate const & gate) const bool Gate::IsValid() const { - return m_weight != kInvalidWeight && !m_stopIds.empty(); + return m_weight != kInvalidWeight && (m_entrance || m_exit) && !m_stopIds.empty(); } // Edge ------------------------------------------------------------------------------------------- @@ -170,6 +170,12 @@ bool Edge::IsEqualForTesting(Edge const & edge) const bool Edge::IsValid() const { + if (m_transfer && (m_lineId != kInvalidLineId || !m_shapeIds.empty())) + return false; + + if (!m_transfer && m_lineId == kInvalidLineId) + return false; + return m_stop1Id != kInvalidStopId && m_stop2Id != kInvalidStopId && m_weight != kInvalidWeight && m_lineId != kInvalidLineId; } @@ -241,7 +247,7 @@ bool Shape::IsEqualForTesting(Shape const & shape) const bool Shape::IsValid() const { return m_id != kInvalidShapeId && m_stop1_id != kInvalidStopId && m_stop2_id != kInvalidStopId && - !m_polyline.empty(); + m_polyline.size() > 1; } // Network ---------------------------------------------------------------------------------------- diff --git a/routing_common/transit_types.hpp b/routing_common/transit_types.hpp index a557b4be9f..62106a0bde 100644 --- a/routing_common/transit_types.hpp +++ b/routing_common/transit_types.hpp @@ -182,6 +182,8 @@ private: // |m_featureId| is feature id of a point feature which represents gates. FeatureId m_featureId = kInvalidFeatureId; // |m_bestPedestrianSegment| is a segment which can be used for pedestrian routing to leave and enter the gate. + // The segment may be invalid because of map date. If so there's no pedestrian segment which can be used + // to reach the gate. SingleMwmSegment m_bestPedestrianSegment; bool m_entrance = true; bool m_exit = true;