From ec7934ea336f0455519bc83666460514eb74ce25 Mon Sep 17 00:00:00 2001 From: FILLAU Jean-Maxime Date: Mon, 22 May 2017 12:10:43 +0200 Subject: [PATCH] Change qi::lit for qi::symbols for the sides parameter parser. Refactor code : - Suppress StartSide Enum - Change Side Structure for Enum Signed-off-by: FILLAU Jean-Maxime --- .../contiguous_internalmem_datafacade.hpp | 8 ++-- include/engine/datafacade/datafacade_base.hpp | 2 +- include/engine/geospatial_query.hpp | 11 ++--- include/engine/plugins/plugin_base.hpp | 8 ++-- include/engine/side.hpp | 40 +------------------ .../server/api/base_parameters_grammar.hpp | 23 +++++------ unit_tests/mocks/mock_datafacade.hpp | 5 +-- unit_tests/server/parameters_io.hpp | 2 +- unit_tests/server/parameters_parser.cpp | 27 ++++++------- 9 files changed, 40 insertions(+), 86 deletions(-) diff --git a/include/engine/datafacade/contiguous_internalmem_datafacade.hpp b/include/engine/datafacade/contiguous_internalmem_datafacade.hpp index 03bee1b05..a66d53fd9 100644 --- a/include/engine/datafacade/contiguous_internalmem_datafacade.hpp +++ b/include/engine/datafacade/contiguous_internalmem_datafacade.hpp @@ -714,14 +714,14 @@ class ContiguousInternalMemoryDataFacadeBase : public BaseDataFacade input_coordinate, max_results, max_distance, bearing, bearing_range); } - std::pair NearestPhantomNodeWithAlternativeFromBigComponent( - const util::Coordinate input_coordinate, - const engine::SideValue side_value) const override final + std::pair + NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, + const engine::Side side) const override final { BOOST_ASSERT(m_geospatial_query.get()); return m_geospatial_query->NearestPhantomNodeWithAlternativeFromBigComponent( - input_coordinate, side_value); + input_coordinate, side); } std::pair NearestPhantomNodeWithAlternativeFromBigComponent( diff --git a/include/engine/datafacade/datafacade_base.hpp b/include/engine/datafacade/datafacade_base.hpp index 48f9bbe9c..cd467478e 100644 --- a/include/engine/datafacade/datafacade_base.hpp +++ b/include/engine/datafacade/datafacade_base.hpp @@ -120,7 +120,7 @@ class BaseDataFacade virtual std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, - const engine::SideValue side_value) const = 0; + const engine::Side side) const = 0; virtual std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, const double max_distance) const = 0; diff --git a/include/engine/geospatial_query.hpp b/include/engine/geospatial_query.hpp index e9e612f37..fff633f7f 100644 --- a/include/engine/geospatial_query.hpp +++ b/include/engine/geospatial_query.hpp @@ -212,19 +212,20 @@ template class GeospatialQuery // a second phantom node is return that is the nearest coordinate in a big component. std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, - const engine::SideValue side_value) const + const engine::Side side) const { bool has_small_component = false; bool has_big_component = false; auto results = rtree.Nearest( input_coordinate, - [this, &side_value, &input_coordinate, &has_big_component, &has_small_component](const CandidateSegment &segment) { + [this, &side, &input_coordinate, &has_big_component, &has_small_component]( + const CandidateSegment &segment) { auto use_segment = (!has_small_component || (!has_big_component && !IsTinyComponent(segment))); auto use_directions = std::make_pair(use_segment, use_segment); bool isOnewaySegment = !(segment.data.forward_segment_id.enabled && segment.data.reverse_segment_id.enabled); - if (!isOnewaySegment && side_value != BOTH) + if (!isOnewaySegment && side != BOTH) { // Check the counter clockwise // @@ -241,8 +242,8 @@ template class GeospatialQuery // if drive left // input_coordinate_is_at_right = !input_coordinate_is_at_right - // We reverse goCountrySide if side_value is OPPOSITE - if (side_value == OPPOSITE) + // We reverse goCountrySide if side is OPPOSITE + if (side == OPPOSITE) input_coordinate_is_at_right = !input_coordinate_is_at_right; // Apply the side. diff --git a/include/engine/plugins/plugin_base.hpp b/include/engine/plugins/plugin_base.hpp index 969bd1a28..4717a4733 100644 --- a/include/engine/plugins/plugin_base.hpp +++ b/include/engine/plugins/plugin_base.hpp @@ -231,11 +231,11 @@ class BasePlugin BOOST_ASSERT(parameters.IsValid()); for (const auto i : util::irange(0UL, parameters.coordinates.size())) { - SideValue sideValue = engine::SideValue::BOTH; + Side side = engine::Side::BOTH; // TODO init at SIDE for test - // SideValue sideValue = engine::SideValue::DEFAULT; + // SideValue side = engine::SideValue::DEFAULT; if (use_sides && parameters.sides[i]) - sideValue = parameters.sides[i]->side; + side = parameters.sides[i].get(); if (use_hints && parameters.hints[i] && parameters.hints[i]->IsValid(parameters.coordinates[i], facade)) @@ -277,7 +277,7 @@ class BasePlugin { phantom_node_pairs[i] = facade.NearestPhantomNodeWithAlternativeFromBigComponent( - parameters.coordinates[i], sideValue); + parameters.coordinates[i], side); } } diff --git a/include/engine/side.hpp b/include/engine/side.hpp index 6866bff85..b1cf8a666 100644 --- a/include/engine/side.hpp +++ b/include/engine/side.hpp @@ -28,56 +28,18 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifndef OSRM_ENGINE_SIDE_HPP #define OSRM_ENGINE_SIDE_HPP -#include - namespace osrm { namespace engine { -enum SideValue +enum Side { DEFAULT, OPPOSITE, BOTH }; - -struct Side -{ - SideValue side; - static SideValue fromString(const std::string &str) - { - if (str == "d") - return DEFAULT; - else if (str == "o") - return OPPOSITE; - else - return BOTH; - } - static std::string toString(const Side &side) - { - switch(side.side) - { - case(DEFAULT) : - return "0"; - case(OPPOSITE) : - return "d"; - case(BOTH) : - return "b"; - default : - //TODO I don't know what to do here. - return "b"; - } - } -}; - -inline bool operator==(const Side lhs, const Side rhs) -{ - return lhs.side == rhs.side; -} -inline bool operator!=(const Side lhs, const Side rhs) { return !(lhs == rhs); } } } - #endif diff --git a/include/server/api/base_parameters_grammar.hpp b/include/server/api/base_parameters_grammar.hpp index 75686e85d..7d57e3d9c 100644 --- a/include/server/api/base_parameters_grammar.hpp +++ b/include/server/api/base_parameters_grammar.hpp @@ -99,16 +99,6 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar base_parameters.bearings.push_back(std::move(bearing)); }; - const auto add_side = [](engine::api::BaseParameters &base_parameters, - const boost::optional &side_string) { - boost::optional side; - if (side_string) - { - side = engine::Side{engine::Side::fromString(side_string.get())}; - } - base_parameters.sides.push_back(std::move(side)); - }; - polyline_chars = qi::char_("a-zA-Z0-9_.--[]{}@?|\\%~`^"); base64_char = qi::char_("a-zA-Z0-9--_="); unlimited_rule = qi::lit("unlimited")[qi::_val = std::numeric_limits::infinity()]; @@ -159,13 +149,16 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar qi::lit("bearings=") > (-(qi::short_ > ',' > qi::short_))[ph::bind(add_bearing, qi::_r1, qi::_1)] % ';'; - sides_rule = qi::lit("sides=") > - (-qi::as_string[qi::char_("a-zA")])[ph::bind(add_side, qi::_r1, qi::_1)] % ';'; + side_type.add("d", engine::Side::DEFAULT)("o", engine::Side::OPPOSITE)("b", engine::Side::BOTH); + sides_rule = + qi::lit("sides=") > + (-side_type % ';')[ph::bind(&engine::api::BaseParameters::sides, qi::_r1) = qi::_1]; base_rule = radiuses_rule(qi::_r1) // | hints_rule(qi::_r1) // | bearings_rule(qi::_r1) // - | generate_hints_rule(qi::_r1) | sides_rule(qi::_r1); + | generate_hints_rule(qi::_r1) + | sides_rule(qi::_r1); } protected: @@ -176,9 +169,9 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar qi::rule bearings_rule; qi::rule radiuses_rule; qi::rule hints_rule; - qi::rule sides_rule; qi::rule generate_hints_rule; + qi::rule sides_rule; qi::rule bearing_rule; qi::rule location_rule; @@ -188,6 +181,8 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar qi::rule polyline_chars; qi::rule unlimited_rule; qi::real_parser double_; + + qi::symbols side_type; }; } } diff --git a/unit_tests/mocks/mock_datafacade.hpp b/unit_tests/mocks/mock_datafacade.hpp index 9b695ffe6..972b78042 100644 --- a/unit_tests/mocks/mock_datafacade.hpp +++ b/unit_tests/mocks/mock_datafacade.hpp @@ -146,9 +146,8 @@ class MockBaseDataFacade : public engine::datafacade::BaseDataFacade } std::pair - NearestPhantomNodeWithAlternativeFromBigComponent( - const util::Coordinate /*input_coordinate*/, - const engine::SideValue /*side_value*/) const override + NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate /*input_coordinate*/, + const engine::Side /*side*/) const override { return {}; } diff --git a/unit_tests/server/parameters_io.hpp b/unit_tests/server/parameters_io.hpp index da706eb75..e044cbe77 100644 --- a/unit_tests/server/parameters_io.hpp +++ b/unit_tests/server/parameters_io.hpp @@ -57,7 +57,7 @@ inline std::ostream &operator<<(std::ostream &out, Bearing bearing) inline std::ostream &operator<<(std::ostream &out, Side side) { - out << Side::toString(side); + out << side; return out; } } diff --git a/unit_tests/server/parameters_parser.cpp b/unit_tests/server/parameters_parser.cpp index a4cc2069c..d8f8eab84 100644 --- a/unit_tests/server/parameters_parser.cpp +++ b/unit_tests/server/parameters_parser.cpp @@ -46,7 +46,7 @@ BOOST_AUTO_TEST_CASE(invalid_route_urls) BOOST_CHECK_EQUAL(testInvalidOptions("1,2;3,4?overview=false&radiuses=foo"), 32UL); BOOST_CHECK_EQUAL(testInvalidOptions("1,2;3,4?overview=false&sides=foo"), - 30UL); + 29UL); BOOST_CHECK_EQUAL(testInvalidOptions("1,2;3,4?overview=false&hints=foo"), 29UL); BOOST_CHECK_EQUAL(testInvalidOptions("1,2;3,4?overview=false&hints=;;; ;"), @@ -400,22 +400,19 @@ BOOST_AUTO_TEST_CASE(valid_route_urls) BOOST_CHECK_EQUAL(result_17->annotations, true); std::vector> sides_18 = { - boost::none, - engine::Side{engine::SideValue::DEFAULT}, - engine::Side{engine::SideValue::BOTH}, - engine::Side{engine::SideValue::OPPOSITE}, + boost::none, engine::Side::DEFAULT, engine::Side::BOTH, engine::Side::OPPOSITE, }; RouteParameters reference_18{false, - false, - false, - RouteParameters::GeometriesType::Polyline, - RouteParameters::OverviewType::Simplified, - boost::optional{}, - coords_3, - std::vector>{}, - std::vector>{}, - std::vector>{}, - sides_18}; + false, + false, + RouteParameters::GeometriesType::Polyline, + RouteParameters::OverviewType::Simplified, + boost::optional{}, + coords_3, + std::vector>{}, + std::vector>{}, + std::vector>{}, + sides_18}; auto result_18 = parseParameters("1,2;3,4;5,6;7,8?steps=false&sides=;d;b;o"); BOOST_CHECK(result_18);