[routing] Review fixes

7 files changed, 137 insertions, 102 deletions

diff --git a/routing/CMakeLists.txt b/routing/CMakeLists.txt
index a6b1552e3d..50ed852816 100644
--- a/routing/CMakeLists.txt
+++ b/routing/CMakeLists.txt
@@ -10,6 +10,8 @@ set(
   async_router.cpp
   async_router.hpp
   base/astar_algorithm.hpp
+  base/astar_progress.cpp
+  base/astar_progress.hpp
   base/astar_weight.hpp
   base/followed_polyline.cpp
   base/followed_polyline.hpp
diff --git a/routing/base/astar_progress.cpp b/routing/base/astar_progress.cpp
new file mode 100644
index 0000000000..51c91877d3
--- /dev/null
+++ b/routing/base/astar_progress.cpp
@@ -0,0 +1,107 @@
+#include "routing/base/astar_progress.hpp"
+
+#include "geometry/mercator.hpp"
+
+#include "base/assert.hpp"
+#include "base/math.hpp"
+
+#include <algorithm>
+#include <iterator>
+
+namespace routing
+{
+// AStarSubProgress ----------------------------------------------------------------------
+
+AStarSubProgress::AStarSubProgress(m2::PointD const & start, m2::PointD const & finish,
+                                   double contributionCoef)
+  : m_contributionCoef(contributionCoef), m_startPoint(start), m_finishPoint(finish)
+{
+  ASSERT_GREATER(m_contributionCoef, 0.0, ());
+
+  m_fullDistance = MercatorBounds::DistanceOnEarth(start, finish);
+  m_forwardDistance = m_fullDistance;
+  m_backwardDistance = m_fullDistance;
+}
+
+AStarSubProgress::AStarSubProgress(double contributionCoef)
+    : m_contributionCoef(contributionCoef)
+{
+  ASSERT_NOT_EQUAL(m_contributionCoef, 0.0, ());
+}
+
+double AStarSubProgress::UpdateProgress(m2::PointD const & current, m2::PointD const & finish)
+{
+  ASSERT_NOT_EQUAL(m_fullDistance, 0.0, ());
+
+  double const dist = MercatorBounds::DistanceOnEarth(current, finish);
+  double & toUpdate = finish == m_finishPoint ? m_forwardDistance : m_backwardDistance;
+
+  toUpdate = std::min(toUpdate, dist);
+
+  double part = 2.0 - (m_forwardDistance + m_backwardDistance) / m_fullDistance;
+  part = base::clamp(part, 0.0, 1.0);
+  double const newProgress =  m_contributionCoef * part;
+
+  m_currentProgress = std::max(newProgress, m_currentProgress);
+
+  return m_currentProgress;
+}
+
+double AStarSubProgress::UpdateProgress(double subSubProgressValue)
+{
+  return m_currentProgress + m_contributionCoef * subSubProgressValue;
+}
+
+void AStarSubProgress::Flush(double progress)
+{
+  m_currentProgress += m_contributionCoef * progress;
+}
+
+double AStarSubProgress::GetMaxContribution() const { return m_contributionCoef; }
+
+// AStarProgress -------------------------------------------------------------------------
+
+AStarProgress::AStarProgress()
+{
+  m_subProgresses.emplace_back(AStarSubProgress(1.0));
+}
+
+AStarProgress::~AStarProgress()
+{
+  CHECK(std::next(m_subProgresses.begin()) == m_subProgresses.end(), ());
+}
+
+void AStarProgress::AppendSubProgress(AStarSubProgress const & subProgress)
+{
+  m_subProgresses.emplace_back(subProgress);
+}
+
+void AStarProgress::EraseLastSubProgress()
+{
+  ASSERT(m_subProgresses.begin() != m_subProgresses.end(), ());
+  ASSERT(m_subProgresses.begin() != std::prev(m_subProgresses.end()), ());
+
+  auto prevLast = std::prev(std::prev(m_subProgresses.end()));
+  prevLast->Flush(m_subProgresses.back().GetMaxContribution());
+
+  CHECK(!m_subProgresses.empty(), ());
+  auto const last = std::prev(m_subProgresses.end());
+  m_subProgresses.erase(last);
+}
+
+double AStarProgress::UpdateProgress(m2::PointD const & current, m2::PointD const & end)
+{
+  return UpdateProgressImpl(m_subProgresses.begin(), current, end) * 100.0;
+}
+
+double AStarProgress::GetLastPercent() const { return m_lastValue * 100.0; }
+
+double AStarProgress::UpdateProgressImpl(ListItem subProgress, m2::PointD const & current,
+                                         m2::PointD const & end)
+{
+  if (std::next(subProgress) != m_subProgresses.end())
+    return subProgress->UpdateProgress(UpdateProgressImpl(std::next(subProgress), current, end));
+
+  return subProgress->UpdateProgress(current, end);
+}
+}  // namespace routing
diff --git a/routing/base/astar_progress.hpp b/routing/base/astar_progress.hpp
index ffcfec0e84..abb9aed486 100644
--- a/routing/base/astar_progress.hpp
+++ b/routing/base/astar_progress.hpp
@@ -1,12 +1,7 @@
 #pragma once
 
-#include "geometry/mercator.hpp"
 #include "geometry/point2d.hpp"
 
-#include "base/assert.hpp"
-#include "base/logging.hpp"
-
-#include <iterator>
 #include <list>
 
 namespace routing
@@ -14,114 +9,49 @@ namespace routing
 class AStarSubProgress
 {
 public:
-  AStarSubProgress(m2::PointD const & start, m2::PointD const & finish, double contributionCoef)
-    : m_contributionCoef(contributionCoef), m_startPoint(start), m_finalPoint(finish)
-  {
-    ASSERT_NOT_EQUAL(m_contributionCoef, 0.0, ());
-
-    m_fullDistance = MercatorBounds::DistanceOnEarth(start, finish);
-    m_forwardDistance = m_fullDistance;
-    m_backwardDistance = m_fullDistance;
-  }
-
-  explicit AStarSubProgress(double contributionCoef)
-      : m_contributionCoef(contributionCoef)
-  {
-    ASSERT_NOT_EQUAL(m_contributionCoef, 0.0, ());
-  }
-
-  double UpdateProgress(m2::PointD const & current, m2::PointD const & end)
-  {
-    ASSERT_NOT_EQUAL(m_fullDistance, 0.0, ());
-
-    double dist = MercatorBounds::DistanceOnEarth(current, end);
-    double & toUpdate = end == m_finalPoint ? m_forwardDistance : m_backwardDistance;
-
-    toUpdate = std::min(toUpdate, dist);
+  AStarSubProgress(m2::PointD const & start, m2::PointD const & finish, double contributionCoef);
 
-    double part = 2.0 - (m_forwardDistance + m_backwardDistance) / m_fullDistance;
-    part = base::clamp(part, 0.0, 1.0);
-    double newProgress =  m_contributionCoef * part;
+  explicit AStarSubProgress(double contributionCoef);
 
-    m_currentProgress = std::max(newProgress, m_currentProgress);
+  double UpdateProgress(m2::PointD const & current, m2::PointD const & finish);
+  /// \brief Some |AStarSubProgress| could have their own subProgresses (next item in
+  /// AStarProgress::m_subProgresses after current), in order to return true progress
+  /// back to the upper level of subProgresses, we should do progress backpropagation of
+  /// subProgress of current subProgress - |subSubProgressValue|
+  double UpdateProgress(double subSubProgressValue);
 
-    return m_currentProgress;
-  }
-
-  double UpdateProgress(double subSubProgressValue)
-  {
-    return m_currentProgress + m_contributionCoef * subSubProgressValue;
-  }
-
-  void Flush(double progress)
-  {
-    m_currentProgress += m_contributionCoef * progress;
-  }
-
-  double GetMaxContribution() const { return m_contributionCoef; }
+  void Flush(double progress);
+  double GetMaxContribution() const;
 
 private:
-
   double m_currentProgress = 0.0;
-
   double m_contributionCoef = 0.0;
-
   double m_fullDistance = 0.0;
-
   double m_forwardDistance = 0.0;
   double m_backwardDistance = 0.0;
 
   m2::PointD m_startPoint;
-  m2::PointD m_finalPoint;
+  m2::PointD m_finishPoint;
 };
 
 class AStarProgress
 {
 public:
+  AStarProgress();
+  ~AStarProgress();
 
-  AStarProgress()
-  {
-    m_subProgresses.emplace_back(AStarSubProgress(1.0));
-  }
-
-  void AppendSubProgress(AStarSubProgress const & subProgress)
-  {
-    m_subProgresses.emplace_back(subProgress);
-  }
-
-  void EraseLastSubProgress()
-  {
-    ASSERT(m_subProgresses.begin() != m_subProgresses.end(), ());
-    ASSERT(m_subProgresses.begin() != std::prev(m_subProgresses.end()), ());
-
-    auto prevLast = std::prev(std::prev(m_subProgresses.end()));
-    prevLast->Flush(m_subProgresses.back().GetMaxContribution());
-
-    CHECK(!m_subProgresses.empty(), ());
-    auto last = std::prev(m_subProgresses.end());
-    m_subProgresses.erase(last);
-  }
-
-  double UpdateProgress(m2::PointD const & current, m2::PointD const & end)
-  {
-    return UpdateProgressImpl(m_subProgresses.begin(), current, end) * 100.0;
-  }
-
-  double GetLastValue() const { return m_lastValue * 100.0; }
+  double GetLastPercent() const;
+  void EraseLastSubProgress();
+  void AppendSubProgress(AStarSubProgress const & subProgress);
+  double UpdateProgress(m2::PointD const & current, m2::PointD const & end);
 
 private:
-
   using ListItem = std::list<AStarSubProgress>::iterator;
 
   double UpdateProgressImpl(ListItem subProgress, m2::PointD const & current,
-                            m2::PointD const & end)
-  {
-    if (std::next(subProgress) != m_subProgresses.end())
-      return subProgress->UpdateProgress(UpdateProgressImpl(std::next(subProgress), current, end));
-
-    return subProgress->UpdateProgress(current, end);
-  }
+                            m2::PointD const & end);
 
+  // This value is in range: [0, 1].
   double m_lastValue = 0.0;
 
   std::list<AStarSubProgress> m_subProgresses;
diff --git a/routing/checkpoints.cpp b/routing/checkpoints.cpp
index 461ebae2ba..66d6f23953 100644
--- a/routing/checkpoints.cpp
+++ b/routing/checkpoints.cpp
@@ -44,7 +44,7 @@ void Checkpoints::PassNextPoint()
   CHECK(!IsFinished(), ());
   ++m_passedIdx;
 }
-double Checkpoints::GetPathLength() const
+double Checkpoints::GetSummaryLengthBetweenPointsMeters() const
 {
   double dist = 0.0;
   for (size_t i = 1; i < m_points.size(); ++i)
diff --git a/routing/checkpoints.hpp b/routing/checkpoints.hpp
index e0e905833c..f9d6f5acbb 100644
--- a/routing/checkpoints.hpp
+++ b/routing/checkpoints.hpp
@@ -28,7 +28,7 @@ public:
   size_t GetNumSubroutes() const { return m_points.size() - 1; }
   bool IsFinished() const { return m_passedIdx >= GetNumSubroutes(); }
 
-  double GetPathLength() const;
+  double GetSummaryLengthBetweenPointsMeters() const;
 
   void PassNextPoint();
 
diff --git a/routing/index_router.cpp b/routing/index_router.cpp
index d18cca0f2c..b0bbd5d5b4 100644
--- a/routing/index_router.cpp
+++ b/routing/index_router.cpp
@@ -394,7 +394,7 @@ RouterResultCode IndexRouter::DoCalculateRoute(Checkpoints const & checkpoints,
   PushPassedSubroutes(checkpoints, subroutes);
   unique_ptr<IndexGraphStarter> starter;
   AStarProgress progress;
-  double const checkpointsLength = checkpoints.GetPathLength();
+  double const checkpointsLength = checkpoints.GetSummaryLengthBetweenPointsMeters();
 
   for (size_t i = checkpoints.GetPassedIdx(); i < checkpoints.GetNumSubroutes(); ++i)
   {
@@ -539,7 +539,7 @@ RouterResultCode IndexRouter::CalculateSubroute(Checkpoints const & checkpoints,
     RoutingResult<JointSegment, RouteWeight> routingResult;
 
     uint32_t visitCount = 0;
-    auto lastValue = progress.GetLastValue();
+    auto lastValue = progress.GetLastPercent();
     auto const onVisitJunctionJoints = [&](JointSegment const & from, JointSegment const & to)
     {
       ++visitCount;
@@ -580,7 +580,7 @@ RouterResultCode IndexRouter::CalculateSubroute(Checkpoints const & checkpoints,
     using Weight = IndexGraphStarter::Weight;
 
     uint32_t visitCount = 0;
-    auto lastValue = progress.GetLastValue();
+    auto lastValue = progress.GetLastPercent();
     if (mode == WorldGraphMode::LeapsOnly)
     {
       AStarSubProgress leapsProgress(checkpoints.GetPoint(subrouteIdx),
@@ -908,17 +908,16 @@ RouterResultCode IndexRouter::ProcessLeapsJoints(vector<Segment> const & input,
     using Edge = IndexGraphStarterJoints<IndexGraphStarter>::Edge;
     using Weight = IndexGraphStarterJoints<IndexGraphStarter>::Weight;
 
-    double contribCoef = static_cast<double>(end - start + 1) /
-                         static_cast<double>(input.size());
-    auto startPoint = starter.GetPoint(input[start], true /* front */);
-    auto endPoint = starter.GetPoint(input[end], true /* front */);
+    auto const contribCoef = static_cast<double>(end - start + 1) / (input.size());
+    auto const startPoint = starter.GetPoint(input[start], true /* front */);
+    auto const endPoint = starter.GetPoint(input[end], true /* front */);
     progress.AppendSubProgress({startPoint, endPoint, contribCoef});
     SCOPE_GUARD(progressGuard, [&progress]() {
       progress.EraseLastSubProgress();
     });
 
     uint32_t visitCount = 0;
-    auto lastValue = progress.GetLastValue();
+    auto lastValue = progress.GetLastPercent();
     auto const onVisitJunctionJoints = [&](JointSegment const & from, JointSegment const & to)
     {
       ++visitCount;
@@ -967,7 +966,6 @@ RouterResultCode IndexRouter::ProcessLeapsJoints(vector<Segment> const & input,
       return true;
     }
 
-
     LOG(LINFO, ("Can not find path",
       "from:",
       MercatorBounds::ToLatLon(starter.GetPoint(input[start], input[start].IsForward())),
diff --git a/routing/index_router.hpp b/routing/index_router.hpp
index 4ff70ffcf6..d071054ba6 100644
--- a/routing/index_router.hpp
+++ b/routing/index_router.hpp
@@ -26,8 +26,6 @@
 
 #include "geometry/tree4d.hpp"
 
-#include "base/astar_progress.hpp"
-
 #include <functional>
 #include <memory>
 #include <set>