From 0d68e6786936cb4c2962a9367b6986e95ca10035 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Mon, 30 Oct 2017 18:07:57 +0300 Subject: [PATCH] Review fixes. --- generator/transit_generator.cpp | 36 +++++++++++++++++++-------------- generator/transit_generator.hpp | 27 +++++++++++++------------ routing/index_router.cpp | 7 ++++--- routing/index_router.hpp | 4 ++-- routing/routing_exceptions.hpp | 1 + 5 files changed, 42 insertions(+), 33 deletions(-) diff --git a/generator/transit_generator.cpp b/generator/transit_generator.cpp index c6ba25ac56..4fab82ce70 100644 --- a/generator/transit_generator.cpp +++ b/generator/transit_generator.cpp @@ -6,6 +6,7 @@ #include "traffic/traffic_cache.hpp" +#include "routing/routing_exceptions.hpp" #include "routing/index_router.hpp" #include "routing_common/transit_serdes.hpp" @@ -154,7 +155,6 @@ private: bool m_isSorted = true; }; - /// \returns ref to a stop at |stops| by |stopId|. Stop const & FindStopById(vector const & stops, StopId stopId) { @@ -180,12 +180,18 @@ string GetMwmPath(string const & mwmDir, string const & countryId) return my::JoinFoldersToPath(mwmDir, countryId + DATA_FILE_EXTENSION); } -void LoadBorders(string const & dir, string const countryId, vector & borders) +void LoadBorders(string const & dir, string const & countryId, vector & borders) { string const polyFile = my::JoinPath(dir, BORDERS_DIR, countryId + BORDERS_EXTENSION); borders.clear(); osm::LoadBorders(polyFile, borders); } + +template +void Append(vector const & src, vector & dst) +{ + dst.insert(dst.end(), src.begin(), src.end()); +} } // namespace namespace routing @@ -295,13 +301,13 @@ void GraphData::SerializeToMwm(string const & mwmPath) const void GraphData::Append(GraphData const & rhs) { - m_stops.insert(m_stops.begin(), rhs.m_stops.begin(), rhs.m_stops.end()); - m_gates.insert(m_gates.begin(), rhs.m_gates.begin(), rhs.m_gates.end()); - m_edges.insert(m_edges.begin(), rhs.m_edges.begin(), rhs.m_edges.end()); - m_transfers.insert(m_transfers.begin(), rhs.m_transfers.begin(), rhs.m_transfers.end()); - m_lines.insert(m_lines.begin(), rhs.m_lines.begin(), rhs.m_lines.end()); - m_shapes.insert(m_shapes.begin(), rhs.m_shapes.begin(), rhs.m_shapes.end()); - m_networks.insert(m_networks.begin(), rhs.m_networks.begin(), rhs.m_networks.end()); + ::Append(rhs.m_stops, m_stops); + ::Append(rhs.m_gates, m_gates); + ::Append(rhs.m_edges, m_edges); + ::Append(rhs.m_transfers, m_transfers); + ::Append(rhs.m_lines, m_lines); + ::Append(rhs.m_shapes, m_shapes); + ::Append(rhs.m_networks, m_networks); } void GraphData::Clear() @@ -326,7 +332,7 @@ void GraphData::Sort() Visit(v); } -void GraphData::CalculateEdgeWeight() +void GraphData::CalculateEdgeWeights() { CHECK(is_sorted(m_stops.cbegin(), m_stops.cend()), ()); @@ -342,7 +348,7 @@ void GraphData::CalculateEdgeWeight() } } -void GraphData::CalculateBestPedestrianSegment(string const & mwmPath, string const & countryId) +void GraphData::CalculateBestPedestrianSegments(string const & mwmPath, string const & countryId) { // Creating IndexRouter. SingleMwmIndex index(mwmPath); @@ -389,9 +395,9 @@ void GraphData::CalculateBestPedestrianSegment(string const & mwmPath, string co bestSegment.GetFeatureId(), bestSegment.GetSegmentIdx(), bestSegment.IsForward())); } } - catch (RootException const & e) + catch (MwmIsNotAlifeException const & e) { - LOG(LDEBUG, ("Point of a gate belongs to several mwms or doesn't belong to any mwm. Gate:", + LOG(LERROR, ("Point of a gate belongs to several mwms or doesn't belong to any mwm. Gate:", gate, e.what())); } } @@ -446,9 +452,9 @@ void DeserializeFromJson(OsmIdToFeatureIdsMap const & mapping, void ProcessGraph(string const & mwmPath, string const & countryId, OsmIdToFeatureIdsMap const & osmIdToFeatureIdsMap, GraphData & data) { - data.CalculateBestPedestrianSegment(mwmPath, countryId); + data.CalculateBestPedestrianSegments(mwmPath, countryId); data.Sort(); - data.CalculateEdgeWeight(); + data.CalculateEdgeWeights(); CHECK(data.IsValid(), (mwmPath)); } diff --git a/generator/transit_generator.hpp b/generator/transit_generator.hpp index 89b9ee8f0d..ecafdf1518 100644 --- a/generator/transit_generator.hpp +++ b/generator/transit_generator.hpp @@ -29,9 +29,10 @@ class DeserializerFromJson public: DeserializerFromJson(json_struct_t* node, OsmIdToFeatureIdsMap const & osmIdToFeatureIds); - template - typename std::enable_if::value || std::is_enum::value || std::is_same::value>::type - operator()(T & t, char const * name = nullptr) + template + typename std::enable_if::value || std::is_enum::value || + std::is_same::value>::type + operator()(T & t, char const * name = nullptr) { GetField(t, name); return; @@ -60,8 +61,9 @@ public: } } - template - typename std::enable_if::value>::type operator()(T & t, char const * name = nullptr) + template + typename std::enable_if::value>::type operator()(T & t, + char const * name = nullptr) { if (name != nullptr && json_is_object(m_node)) { @@ -107,7 +109,6 @@ private: class GraphData { public: - // @todo(bykoianko) All the methods below should be rewrite with the help of visitors. void DeserializeFromJson(my::Json const & root, OsmIdToFeatureIdsMap const & mapping); void SerializeToMwm(std::string const & mwmPath) const; void Append(GraphData const & rhs); @@ -119,10 +120,10 @@ public: /// \brief Calculates and updates |m_edges| by adding valid value for Edge::m_weight /// if it's not valid. /// \note |m_stops|, Edge::m_stop1Id and Edge::m_stop2Id in |m_edges| should be valid before call. - void CalculateEdgeWeight(); + void CalculateEdgeWeights(); /// \brief Calculates best pedestrian segment for every gate in |m_gates|. /// \note All gates in |m_gates| should have a valid |m_point| field before the call. - void CalculateBestPedestrianSegment(string const & mwmPath, string const & countryId); + void CalculateBestPedestrianSegments(std::string const & mwmPath, std::string const & countryId); /// \brief Removes some items from all the class fields if they outside |mwmBorders|. /// @todo(bykoinko) Certain rules which is used to clip the transit graph should be described here. void ClipGraphByMwm(std::vector const & mwmBorders); @@ -157,13 +158,13 @@ private: /// \brief Fills |data| according to a transit graph (|transitJsonPath|). /// \note Some of fields of |data| contains feature ids of a certain mwm. These fields are filled -/// iff the mapping (|osmIdToFeatureIdsPath|) contains them. If not the fields have default value. -void DeserializeFromJson(OsmIdToFeatureIdsMap const & mapping, string const & transitJsonPath, +/// iff the mapping (|osmIdToFeatureIdsPath|) contains them. Otherwise the fields have default value. +void DeserializeFromJson(OsmIdToFeatureIdsMap const & mapping, std::string const & transitJsonPath, GraphData & data); /// \brief Calculates and adds some information to transit graph (|data|) after deserializing /// from json. -void ProcessGraph(string const & mwmPath, string const & countryId, +void ProcessGraph(std::string const & mwmPath, std::string const & countryId, OsmIdToFeatureIdsMap const & osmIdToFeatureIdsMap, GraphData & data); /// \brief Builds the transit section in the mwm. @@ -175,7 +176,7 @@ void ProcessGraph(string const & mwmPath, string const & countryId, /// \note An mwm pointed by |mwmPath| should contain: /// * feature geometry /// * index graph (ROUTING_FILE_TAG) -void BuildTransit(string const & mwmDir, string const & countryId, - string const & osmIdToFeatureIdsPath, string const & transitDir); +void BuildTransit(std::string const & mwmDir, std::string const & countryId, + std::string const & osmIdToFeatureIdsPath, std::string const & transitDir); } // namespace transit } // namespace routing diff --git a/routing/index_router.cpp b/routing/index_router.cpp index fd5d4db0fe..a76ebd8419 100644 --- a/routing/index_router.cpp +++ b/routing/index_router.cpp @@ -4,6 +4,7 @@ #include "routing/base/astar_progress.hpp" #include "routing/base/routing_result.hpp" #include "routing/bicycle_directions.hpp" +#include "routing/routing_exceptions.hpp" #include "routing/fake_ending.hpp" #include "routing/index_graph.hpp" #include "routing/index_graph_loader.hpp" @@ -317,8 +318,8 @@ IndexRouter::IndexRouter(VehicleType vehicleType, bool loadAltitudes, CHECK(m_directionsEngine, ()); } -bool IndexRouter::FindBestSegmentInSingleMwm(m2::PointD const &point, m2::PointD const &direction, - bool isOutgoing, Segment &bestSegment) +bool IndexRouter::FindBestSegmentInSingleMwm(m2::PointD const & point, m2::PointD const & direction, + bool isOutgoing, Segment & bestSegment) { auto worldGraph = MakeWorldGraph(); worldGraph->SetMode(WorldGraph::Mode::SingleMwm); @@ -692,7 +693,7 @@ bool IndexRouter::FindBestSegment(m2::PointD const & point, m2::PointD const & d auto const file = platform::CountryFile(m_countryFileFn(point)); MwmSet::MwmHandle handle = m_index.GetMwmHandleByCountryFile(file); if (!handle.IsAlive()) - MYTHROW(RoutingException, ("Can't get mwm handle for", file)); + MYTHROW(MwmIsNotAlifeException, ("Can't get mwm handle for", file)); auto const mwmId = MwmSet::MwmId(handle.GetInfo()); NumMwmId const numMwmId = m_numMwmIds->GetId(file); diff --git a/routing/index_router.hpp b/routing/index_router.hpp index 3eb579a52c..e8ad0897c2 100644 --- a/routing/index_router.hpp +++ b/routing/index_router.hpp @@ -63,8 +63,8 @@ public: shared_ptr numMwmIds, unique_ptr> numMwmTree, traffic::TrafficCache const & trafficCache, Index & index); - bool FindBestSegmentInSingleMwm(m2::PointD const &point, m2::PointD const &direction, - bool isOutgoing, Segment &bestSegment); + bool FindBestSegmentInSingleMwm(m2::PointD const & point, m2::PointD const & direction, + bool isOutgoing, Segment & bestSegment); // IRouter overrides: std::string GetName() const override { return m_name; } diff --git a/routing/routing_exceptions.hpp b/routing/routing_exceptions.hpp index 7f64ba51e7..b41c78948a 100644 --- a/routing/routing_exceptions.hpp +++ b/routing/routing_exceptions.hpp @@ -6,4 +6,5 @@ namespace routing { DECLARE_EXCEPTION(CorruptedDataException, RootException); DECLARE_EXCEPTION(RoutingException, RootException); +DECLARE_EXCEPTION(MwmIsNotAlifeException, RootException); } // namespace routing