From 616bf657bb944541770a9c75ca795a5953cd198c Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 14 Jun 2017 18:22:39 +0300 Subject: [PATCH] Review fixes. --- routing/route.cpp | 21 +++++++------- routing/route.hpp | 56 ++++++++++++++++++------------------- routing/routing_session.cpp | 4 +-- 3 files changed, 40 insertions(+), 41 deletions(-) diff --git a/routing/route.cpp b/routing/route.cpp index 43d14b1a2f..9f2c750832 100644 --- a/routing/route.cpp +++ b/routing/route.cpp @@ -470,14 +470,12 @@ void Route::AppendRoute(Route const & route) // This implementation is valid for one subroute which is equal to the route. size_t Route::GetSubrouteCount() const { return IsValid() ? 1 : 0; } -void Route::GetSubrouteInfo(size_t subrouteInx, vector & info) const +void Route::GetSubrouteInfo(size_t segmentIdx, vector & info) const { - CHECK_LESS(subrouteInx, GetSubrouteCount(), ()); + CHECK_LESS(segmentIdx, GetSubrouteCount(), ()); + CHECK(IsValid(), ()); info.clear(); - if (!IsValid()) - return; - vector const & points = m_poly.GetPolyline().GetPoints(); size_t const polySz = m_poly.GetPolyline().GetSize(); @@ -505,6 +503,7 @@ void Route::GetSubrouteInfo(size_t subrouteInx, vector & inf size_t timeIdx = (m_times[0].first ? 1 : 0); double distFromBeginningMeters = 0.0; double distFromBeginningMerc = 0.0; + info.reserve(polySz - 1); for (size_t i = 1; i < points.size(); ++i) { TurnItem turn; @@ -530,16 +529,16 @@ void Route::GetSubrouteInfo(size_t subrouteInx, vector & inf } } -Route::SubrouteSettings const Route::GetSubrouteSettings(size_t subrouteInx) const +Route::SubrouteSettings const Route::GetSubrouteSettings(size_t segmentIdx) const { - CHECK_LESS(subrouteInx, GetSubrouteCount(), ()); - return SubrouteSettings(m_routingSettings, m_router, m_subrouteId); + CHECK_LESS(segmentIdx, GetSubrouteCount(), ()); + return SubrouteSettings(m_routingSettings, m_router, m_subrouteUid); } -void Route::SetSubrouteId(size_t subrouteInx, uint64_t subrouteId) +void Route::SetSubrouteUid(size_t segmentIdx, SubrouteUid subrouteUid) { - CHECK_LESS(subrouteInx, GetSubrouteCount(), ()); - m_subrouteId = subrouteId; + CHECK_LESS(segmentIdx, GetSubrouteCount(), ()); + m_subrouteUid = subrouteUid; } string DebugPrint(Route const & r) diff --git a/routing/route.hpp b/routing/route.hpp index 4f97dafd00..edc7b36c31 100644 --- a/routing/route.hpp +++ b/routing/route.hpp @@ -28,6 +28,8 @@ namespace location namespace routing { +using SubrouteUid = uint64_t; +SubrouteUid constexpr kInvalidSubrouteId = std::numeric_limits::max(); class Route { @@ -38,19 +40,17 @@ public: using TStreetItem = pair; using TStreets = vector; - static uint64_t constexpr kInvalidId = std::numeric_limits::max(); - /// \brief The route is composed of one or several subroutes. Every subroute is composed of segments. /// For every Segment is kept some attributes in the structure SegmentInfo. - struct SegmentInfo + struct SegmentInfo final { SegmentInfo(Segment const & segment, turns::TurnItem const & turn, Junction const & junction, - std::string const & streetName, double distFromBeginningMeters, double distFromBeginningMerc, + std::string const & street, double distFromBeginningMeters, double distFromBeginningMerc, double timeFromBeginningS, traffic::SpeedGroup traffic) : m_segment(segment) , m_turn(turn) , m_junction(junction) - , m_streetName(streetName) + , m_street(street) , m_distFromBeginningMeters(distFromBeginningMeters) , m_distFromBeginningMerc(distFromBeginningMerc) , m_timeFromBeginningS(timeFromBeginningS) @@ -58,35 +58,35 @@ public: { } - Segment m_segment; + Segment const m_segment; /// Turn (maneuver) information for the turn next to the |m_segment| if any. /// If not |m_turn::m_turn| is equal to TurnDirection::NoTurn. - turns::TurnItem m_turn; - Junction m_junction; ///< The front point of segment. - /// Street name of |m_segment| if any. Otherwise |m_streetName| is empty. - std::string m_streetName; + turns::TurnItem const m_turn; + Junction const m_junction; ///< The front point of segment. + /// Street name of |m_segment| if any. Otherwise |m_street| is empty. + std::string const m_street; /// Distance from the route beginning to the farthest end of |m_segment| in meters. - double m_distFromBeginningMeters = 0.0; + double const m_distFromBeginningMeters = 0.0; /// Distance from the route beginning to the farthest end of |m_segment| in mercator. - double m_distFromBeginningMerc = 0.0; + double const m_distFromBeginningMerc = 0.0; /// ETA from the route beginning in seconds to reach the farthest from the route beginning end of |m_segment|. - double m_timeFromBeginningS = 0.0; - traffic::SpeedGroup m_traffic = traffic::SpeedGroup::Unknown; + double const m_timeFromBeginningS = 0.0; + traffic::SpeedGroup const m_traffic = traffic::SpeedGroup::Unknown; }; /// \brief For every subroute some attributes are kept the following stucture. - struct SubrouteSettings + struct SubrouteSettings final { - SubrouteSettings(RoutingSettings const & routingSettings, string const & router, uint64_t id) + SubrouteSettings(RoutingSettings const & routingSettings, string const & router, SubrouteUid id) : m_routingSettings(routingSettings), m_router(router), m_id(id) { } - RoutingSettings m_routingSettings; - string m_router; + RoutingSettings const m_routingSettings; + string const m_router; /// Some subsystems (for example drape) which is used Route class need to have an id of any subroute. /// This subsystems may set the id and then use it. The id is kept in |m_id|. - uint64_t m_id = kInvalidId; + SubrouteUid const m_id = kInvalidSubrouteId; }; explicit Route(string const & router) @@ -137,7 +137,7 @@ public: TTurns const & GetTurns() const { return m_turns; } feature::TAltitudes const & GetAltitudes() const { return m_altitudes; } vector const & GetTraffic() const { return m_traffic; } - vector const & GetSegDistanceM() const { return m_poly.GetSegDistanceM(); } + vector const & GetSegDistanceMeters() const { return m_poly.GetSegDistanceM(); } /// \returns distance to all turns in mercator. void GetTurnsDistances(vector & distances) const; bool IsValid() const { return (m_poly.GetPolyline().GetSize() > 1); } @@ -195,7 +195,7 @@ public: size_t GetSubrouteCount() const; /// \brief Fills |info| with full subroute information. - /// \param subrouteInx zero base number of subroute. |subrouteInx| should be less than GetSubrouteCount(); + /// \param segmentIdx zero base number of subroute. |segmentIdx| should be less than GetSubrouteCount(); /// \note |info| is a segment oriented route. Size of |info| is equal to number of points in |m_poly| - 1. /// Class Route is a point oriented route. While this conversion some attributes of zero point will be lost. /// It happens with zero turn for example. @@ -203,17 +203,17 @@ public: /// intermediate points. /// Note. SegmentInfo::m_segment is filled with default Segment instance. /// Note. SegmentInfo::m_streetName is filled with an empty string. - void GetSubrouteInfo(size_t subrouteInx, vector & info) const; + void GetSubrouteInfo(size_t segmentIdx, vector & info) const; - /// \returns Subroute settings by |subRouteInx|. + /// \returns Subroute settings by |segmentIdx|. // @TODO(bykoianko) This method should return SubrouteSettings by reference. Now it returns by value // because of fake implementation. - SubrouteSettings const GetSubrouteSettings(size_t subrouteInx) const; + SubrouteSettings const GetSubrouteSettings(size_t segmentIdx) const; - /// \brief Sets subroute id by |subRouteInx|. - /// \note |subrouteId| is a permanent id of a subroute. This id can be used to address to a subroute + /// \brief Sets subroute unique id (|subrouteUid|) by |segmentIdx|. + /// \note |subrouteUid| is a permanent id of a subroute. This id can be used to address to a subroute /// after the route is removed. - void SetSubrouteId(size_t subrouteInx, uint64_t subrouteId); + void SetSubrouteUid(size_t segmentIdx, SubrouteUid subrouteUid); private: friend string DebugPrint(Route const & r); @@ -243,7 +243,7 @@ private: mutable double m_currentTime; // Subroute - uint64_t m_subrouteId = kInvalidId; + SubrouteUid m_subrouteUid = kInvalidSubrouteId; }; } // namespace routing diff --git a/routing/routing_session.cpp b/routing/routing_session.cpp index e3aaa1095b..dfdf2c3525 100644 --- a/routing/routing_session.cpp +++ b/routing/routing_session.cpp @@ -603,7 +603,7 @@ bool RoutingSession::HasRouteAltitudeImpl() const { ASSERT(m_route, ()); - return m_route->GetAltitudes().size() == m_route->GetSegDistanceM().size() + 1; + return m_route->GetAltitudes().size() == m_route->GetSegDistanceMeters().size() + 1; } bool RoutingSession::HasRouteAltitude() const @@ -621,7 +621,7 @@ bool RoutingSession::GetRouteAltitudesAndDistancesM(vector & routeSegDis if (!m_route->IsValid() || !HasRouteAltitudeImpl()) return false; - routeSegDistanceM = m_route->GetSegDistanceM(); + routeSegDistanceM = m_route->GetSegDistanceMeters(); routeAltitudesM.assign(m_route->GetAltitudes().cbegin(), m_route->GetAltitudes().cend()); return true; }