diff --git a/routing/async_router.cpp b/routing/async_router.cpp index fd0c1f1887..08af0bc7d3 100644 --- a/routing/async_router.cpp +++ b/routing/async_router.cpp @@ -10,6 +10,7 @@ #include "base/timer.hpp" #include "std/functional.hpp" +#include "std/utility.hpp" using namespace std; using namespace std::placeholders; @@ -58,7 +59,7 @@ AsyncRouter::RouterDelegateProxy::RouterDelegateProxy(ReadyCallbackOwnership con m_delegate.SetTimeout(timeoutSec); } -void AsyncRouter::RouterDelegateProxy::OnReady(Route & route, RouterResultCode resultCode) +void AsyncRouter::RouterDelegateProxy::OnReady(unique_ptr route, RouterResultCode resultCode) { if (!m_onReady) return; @@ -67,7 +68,7 @@ void AsyncRouter::RouterDelegateProxy::OnReady(Route & route, RouterResultCode r if (m_delegate.IsCancelled()) return; } - m_onReady(route, resultCode); + m_onReady(move(route), resultCode); } void AsyncRouter::RouterDelegateProxy::OnNeedMoreMaps(uint64_t routeId, @@ -314,7 +315,7 @@ void AsyncRouter::CalculateRoute() routeId = ++m_routeCounter; } - Route route(router->GetName(), routeId); + unique_ptr route = make_unique(router->GetName(), routeId); RouterResultCode code; my::Timer timer; @@ -330,7 +331,7 @@ void AsyncRouter::CalculateRoute() // Run basic request. code = router->CalculateRoute(checkpoints, startDirection, adjustToPrevRoute, - delegate->GetDelegate(), route); + delegate->GetDelegate(), *route); elapsedSec = timer.ElapsedSeconds(); // routing time LogCode(code, elapsedSec); @@ -340,16 +341,16 @@ void AsyncRouter::CalculateRoute() code = RouterResultCode::InternalError; LOG(LERROR, ("Exception happened while calculating route:", e.Msg())); SendStatistics(checkpoints.GetStart(), startDirection, checkpoints.GetFinish(), e.Msg()); - delegate->OnReady(route, code); + delegate->OnReady(move(route), code); return; } - SendStatistics(checkpoints.GetStart(), startDirection, checkpoints.GetFinish(), code, route, + SendStatistics(checkpoints.GetStart(), startDirection, checkpoints.GetFinish(), code, *route, elapsedSec); // Draw route without waiting network latency. if (code == RouterResultCode::NoError) - delegate->OnReady(route, code); + delegate->OnReady(move(route), code); bool const needFetchAbsent = (code != RouterResultCode::Cancelled); diff --git a/routing/async_router.hpp b/routing/async_router.hpp index e88554d241..84d897dc77 100644 --- a/routing/async_router.hpp +++ b/routing/async_router.hpp @@ -43,9 +43,8 @@ public: /// @param readyCallback function to return routing result /// @param progressCallback function to update the router progress /// @param timeoutSec timeout to cancel routing. 0 is infinity. - // @TODO(bykoianko) Gather |readyCallback| with passing ownership of unique_ptr, - // |needMoreMapsCallback| and |removeRouteCallback| to one delegate. No need to add - // |progressCallback| to the delegate. + // @TODO(bykoianko) Gather |readyCallback|, |needMoreMapsCallback| and |removeRouteCallback| + // to one delegate. No need to add |progressCallback| to the delegate. void CalculateRoute(Checkpoints const & checkpoints, m2::PointD const & direction, bool adjustToPrevRoute, ReadyCallbackOwnership const & readyCallback, NeedMoreMapsCallback const & needMoreMapsCallback, @@ -88,7 +87,7 @@ private: ProgressCallback const & onProgress, uint32_t timeoutSec); - void OnReady(Route & route, RouterResultCode resultCode); + void OnReady(unique_ptr route, RouterResultCode resultCode); void OnNeedMoreMaps(uint64_t routeId, std::vector const & absentCounties); void OnRemoveRoute(RouterResultCode resultCode); void Cancel(); @@ -101,8 +100,9 @@ private: mutex m_guard; ReadyCallbackOwnership const m_onReady; - // |m_onNeedMoreMaps| may be called after |m_onReady| if there's a faster route, - // but it's necessary to load some more maps to build it. + // |m_onNeedMoreMaps| may be called after |m_onReady| if + // - it's possible to build route only if to load some maps + // - there's a faster route, but it's necessary to load some more maps to build it NeedMoreMapsCallback const m_onNeedMoreMaps; RemoveRouteCallback const m_removeRoute; PointCheckCallback const m_onPointCheck; diff --git a/routing/routing_callbacks.hpp b/routing/routing_callbacks.hpp index 58ff9023bb..fa9d694c2b 100644 --- a/routing/routing_callbacks.hpp +++ b/routing/routing_callbacks.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -42,12 +43,8 @@ using CheckpointCallback = std::function; using NeedMoreMapsCallback = std::function const &)>; using PointCheckCallback = std::function; using ProgressCallback = std::function; -// @TODO(bykoianko) ReadyCallback and ReadyCallbackOwnership callbacks should be gathered -// to one with the following signature: -// std::function, RouterResultCode)> -// That means calling ReadyCallback means passing ownership of ready instance of Route. using ReadyCallback = std::function; -using ReadyCallbackOwnership = std::function; +using ReadyCallbackOwnership = std::function, RouterResultCode)>; using RemoveRouteCallback = std::function; using RouteCallback = std::function; using RoutingStatisticsCallback = std::function const &)>; diff --git a/routing/routing_session.cpp b/routing/routing_session.cpp index c28c7dba44..ceb9c2f18e 100644 --- a/routing/routing_session.cpp +++ b/routing/routing_session.cpp @@ -137,12 +137,13 @@ m2::PointD RoutingSession::GetEndPoint() const return m_checkpoints.GetFinish(); } -void RoutingSession::DoReadyCallback::operator()(Route & route, RouterResultCode e) +void RoutingSession::DoReadyCallback::operator()(unique_ptr route, RouterResultCode e) { threads::MutexGuard guard(m_routeSessionMutexInner); ASSERT(m_rs.m_route, ()); - m_rs.AssignRoute(route, e); + // @TODO(bykoianko) Move |route| to m_rs.AssignRoute() method. + m_rs.AssignRoute(*route, e); m_callback(*m_rs.m_route, e); } diff --git a/routing/routing_session.hpp b/routing/routing_session.hpp index 62a565ad52..ebf33b55f8 100644 --- a/routing/routing_session.hpp +++ b/routing/routing_session.hpp @@ -189,7 +189,7 @@ private: { } - void operator()(Route & route, RouterResultCode e); + void operator()(unique_ptr route, RouterResultCode e); }; // Should be called with locked m_routingSessionMutex. diff --git a/routing/routing_tests/async_router_test.cpp b/routing/routing_tests/async_router_test.cpp index 7633be0dd5..29e09aa8c5 100644 --- a/routing/routing_tests/async_router_test.cpp +++ b/routing/routing_tests/async_router_test.cpp @@ -13,6 +13,7 @@ #include "std/cstdint.hpp" #include "std/mutex.hpp" #include "std/string.hpp" +#include "std/unique_ptr.hpp" #include "std/vector.hpp" using namespace routing; @@ -67,13 +68,14 @@ struct DummyResultCallback uint32_t const m_expected; uint32_t m_called; - DummyResultCallback(uint32_t expectedCalls) : m_expected(expectedCalls), m_called(0) {} + explicit DummyResultCallback(uint32_t expectedCalls) : m_expected(expectedCalls), m_called(0) {} // ReadyCallbackOwnership callback - void operator()(Route & route, RouterResultCode code) + void operator()(unique_ptr route, RouterResultCode code) { + CHECK(route, ()); m_codes.push_back(code); - auto const & absent = route.GetAbsentCountries(); + auto const & absent = route->GetAbsentCountries(); m_absent.emplace_back(absent.begin(), absent.end()); TestAndNotifyReadyCallbacks(); }