From bb5c7823c566ef2de39ba2652005fda7b924fe19 Mon Sep 17 00:00:00 2001
From: Yuri Gorshenin <y@maps.me>
Date: Wed, 22 Apr 2015 19:00:50 +0300
Subject: [PATCH] Review fixes.

---
 routing/road_graph.cpp        | 14 +++++++-------
 routing/road_graph.hpp        |  8 ++++----
 routing/road_graph_router.cpp |  8 ++++++++
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/routing/road_graph.cpp b/routing/road_graph.cpp
index c364618ff1..0064e4a6df 100644
--- a/routing/road_graph.cpp
+++ b/routing/road_graph.cpp
@@ -74,7 +74,7 @@ void IRoadGraph::CrossTurnsLoader::operator()(uint32_t featureId, RoadInfo const
   {
     m2::PointD const & p = roadInfo.m_points[i];
 
-    /// @todo Is this a correct way to compare?
+    // @todo Is this a correct way to compare?
     if (!m2::AlmostEqual(m_cross, p))
       continue;
 
@@ -149,8 +149,8 @@ void IRoadGraph::GetNearestTurns(RoadPos const & pos, TurnsVectorT & turns)
 {
   uint32_t const featureId = pos.GetFeatureId();
 
-  // For fake start and final positions just add vicinity turns as
-  // nearest turns.
+  // For fake start and final positions add vicinity turns as nearest
+  // turns.
   if (featureId == RoadPos::kFakeStartFeatureId)
   {
     turns.insert(turns.end(), m_startVicinityTurns.begin(), m_startVicinityTurns.end());
@@ -178,7 +178,7 @@ void IRoadGraph::GetNearestTurns(RoadPos const & pos, TurnsVectorT & turns)
   AddFakeTurns(pos, roadInfo, m_startVicinityRoadPoss, turns);
   AddFakeTurns(pos, roadInfo, m_finalVicinityRoadPoss, turns);
 
-  // It's also possible to move from a start's or final's vicinity
+  // It is also possible to move from a start's or final's vicinity
   // positions to start or final points.
   for (PossibleTurn const & turn : m_fakeTurns[pos])
     turns.push_back(turn);
@@ -192,7 +192,7 @@ void IRoadGraph::AddFakeTurns(RoadPos const & pos, RoadInfo const & roadInfo,
     if (!vpos.SameRoadSegmentAndDirection(pos))
       continue;
 
-    // It's also possible to move from a road position to start's or
+    // It is also possible to move from a road position to start's or
     // final's vicinity positions if they're on the same road segment.
     PossibleTurn turn;
     turn.m_secondsCovered = TimeBetweenSec(pos.GetSegEndpoint(), vpos.GetSegEndpoint());
@@ -238,11 +238,11 @@ void IRoadGraph::AddFakeTurns(RoadPos const & rp, vector<RoadPos> const & vicini
   {
     PossibleTurn turn;
     turn.m_pos = vrp;
-    /// @todo Do we need other fields? Do we even need m_secondsCovered?
+    // @todo Do we need other fields? Do we even need m_secondsCovered?
     turn.m_secondsCovered = TimeBetweenSec(rp.GetSegEndpoint(), vrp.GetSegEndpoint());
     turns->push_back(turn);
 
-    /// Add a fake turn from a vicincy road position to a fake point.
+    // Add a fake turn from a vicincy road position to a fake point.
     turn.m_pos = rp;
     m_fakeTurns[vrp].push_back(turn);
   }
diff --git a/routing/road_graph.hpp b/routing/road_graph.hpp
index 0f7d995f16..6c9244954f 100644
--- a/routing/road_graph.hpp
+++ b/routing/road_graph.hpp
@@ -39,19 +39,19 @@ public:
   uint32_t GetSegEndPointId() const { return m_segId + (IsForward() ? 1 : 0); }
   m2::PointD const & GetSegEndpoint() const { return m_segEndpoint; }
 
-  bool SameRoadSegmentAndDirection(RoadPos const & r) const
+  inline bool SameRoadSegmentAndDirection(RoadPos const & r) const
   {
     return m_featureId == r.m_featureId && m_segId == r.m_segId;
   }
 
-  bool operator==(RoadPos const & r) const
+  inline bool operator==(RoadPos const & r) const
   {
     return m_featureId == r.m_featureId && m_segId == r.m_segId && m_segEndpoint == r.m_segEndpoint;
   }
 
-  bool operator!=(RoadPos const & r) const { return !(*this == r); }
+  inline bool operator!=(RoadPos const & r) const { return !(*this == r); }
 
-  bool operator<(RoadPos const & r) const
+  inline bool operator<(RoadPos const & r) const
   {
     if (m_featureId != r.m_featureId)
       return m_featureId < r.m_featureId;
diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp
index fd587a9cf9..b6e644b946 100644
--- a/routing/road_graph_router.cpp
+++ b/routing/road_graph_router.cpp
@@ -18,6 +18,14 @@ namespace routing
 {
 namespace
 {
+// TODO (@gorshenin, @pimenov, @ldragunov): MAX_ROAD_CANDIDATES == 2
+// means that only one closest feature (in both directions) will be
+// examined when searching for features in the vicinity of start and
+// final points. It is an oversimplification that is not as easily
+// solved as tuning up this constant because if you set it too high
+// you risk to find a feature that you cannot in fact reach because of
+// an obstacle.  Using only the closest feature minimizes (but not
+// eliminates) this risk.
 size_t const MAX_ROAD_CANDIDATES = 2;
 double const FEATURE_BY_POINT_RADIUS_M = 100.0;
 }  // namespace