From 9cb1f213f43ebfa9f9e0c3b61dafa14e7f5ad062 Mon Sep 17 00:00:00 2001 From: kevin Date: Tue, 11 Feb 2020 17:02:08 +0800 Subject: [PATCH] Fix review comments. --- CMakeLists.txt | 1 + include/mbgl/style/expression/in.hpp | 7 +--- src/mbgl/style/expression/in.cpp | 58 ++++++++++++++-------------- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e16dd9da057..7a8fe30f303 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -565,6 +565,7 @@ add_library( ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/get_covering_stops.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/image.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/image_expression.cpp + ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/in.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/interpolate.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/is_constant.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/is_expression.cpp diff --git a/include/mbgl/style/expression/in.hpp b/include/mbgl/style/expression/in.hpp index ab8e2e9ff2a..6bd57e2833f 100644 --- a/include/mbgl/style/expression/in.hpp +++ b/include/mbgl/style/expression/in.hpp @@ -8,7 +8,7 @@ namespace mbgl { namespace style { namespace expression { -class In : public Expression { +class In final : public Expression { public: In(std::unique_ptr needle_, std::unique_ptr haystack_) : Expression(Kind::In, type::Boolean), needle(std::move(needle_)), haystack(std::move(haystack_)) {} @@ -26,16 +26,13 @@ class In : public Expression { return false; } - std::vector> possibleOutputs() const override { return {nullopt}; } + std::vector> possibleOutputs() const override { return {{true}, {false}}; } std::string getOperator() const override { return "in"; } private: std::unique_ptr needle; std::unique_ptr haystack; - static bool isComparableType(type::Type type); - static bool isComparableRuntimeValue(type::Type type); - static bool isSearchableRuntimeValue(type::Type type); }; } // namespace expression diff --git a/src/mbgl/style/expression/in.cpp b/src/mbgl/style/expression/in.cpp index 05336c7d922..28658b4a313 100644 --- a/src/mbgl/style/expression/in.cpp +++ b/src/mbgl/style/expression/in.cpp @@ -8,15 +8,30 @@ namespace mbgl { namespace style { namespace expression { +namespace { +bool isComparableType(type::Type type) { + return type == type::Boolean || type == type::String || type == type::Number || type == type::Null || + type == type::Value; +} + +bool isComparableRuntimeValue(type::Type type) { + return type == type::Boolean || type == type::String || type == type::Number || type == type::Null; +} + +bool isSearchableRuntimeValue(type::Type type) { + return type == type::String || type.is() || type == type::Null; +} +} + EvaluationResult In::evaluate(const EvaluationContext& params) const { const EvaluationResult evaluatedNeedle = needle->evaluate(params); - const EvaluationResult evaluatedHeystack = haystack->evaluate(params); - if (!evaluatedNeedle) { return evaluatedNeedle.error(); } - if (!evaluatedHeystack) { - return evaluatedHeystack.error(); + + const EvaluationResult evaluatedHaystack = haystack->evaluate(params); + if (!evaluatedHaystack) { + return evaluatedHaystack.error(); } type::Type evaluatedNeedleType = typeOf(*evaluatedNeedle); @@ -25,18 +40,18 @@ EvaluationResult In::evaluate(const EvaluationContext& params) const { toString(evaluatedNeedleType) + " instead."}; } - type::Type evaluatedHeystackType = typeOf(*evaluatedHeystack); - if (!isSearchableRuntimeValue(evaluatedHeystackType)) { + type::Type evaluatedHaystackType = typeOf(*evaluatedHaystack); + if (!isSearchableRuntimeValue(evaluatedHaystackType)) { return EvaluationError{"Expected second argument to be of type array or string, but found " + - toString(evaluatedHeystackType) + " instead."}; + toString(evaluatedHaystackType) + " instead."}; } - if (evaluatedNeedleType == type::Null || evaluatedHeystackType == type::Null) { + if (evaluatedNeedleType == type::Null || evaluatedHaystackType == type::Null) { return EvaluationResult(false); } - if (evaluatedHeystackType == type::String) { - const auto heystackString = evaluatedHeystack->get(); + if (evaluatedHaystackType == type::String) { + const auto haystackString = evaluatedHaystack->get(); std::string needleValue = ""; if (evaluatedNeedleType == type::Boolean) { needleValue = evaluatedNeedle->get() ? "true" : " false"; @@ -47,20 +62,20 @@ EvaluationResult In::evaluate(const EvaluationContext& params) const { needleValue.erase(needleValue.find_last_not_of('0') + 1, std::string::npos); needleValue.erase(needleValue.find_last_not_of('.') + 1, std::string::npos); } - return EvaluationResult(heystackString.find(needleValue) != std::string::npos); + return EvaluationResult(haystackString.find(needleValue) != std::string::npos); } else { - const auto heystackArray = evaluatedHeystack->get>(); + const auto haystackArray = evaluatedHaystack->get>(); bool result = false; if (evaluatedNeedleType == type::Boolean) { auto needleValue = evaluatedNeedle->get(); - result = find(heystackArray.begin(), heystackArray.end(), needleValue) != heystackArray.end(); + result = find(haystackArray.begin(), haystackArray.end(), needleValue) != haystackArray.end(); } else if (evaluatedNeedleType == type::String) { auto needleValue = evaluatedNeedle->get(); - result = find(heystackArray.begin(), heystackArray.end(), needleValue) != heystackArray.end(); + result = find(haystackArray.begin(), haystackArray.end(), needleValue) != haystackArray.end(); } else if (evaluatedNeedleType == type::Number) { auto needleValue = evaluatedNeedle->get(); - result = find(heystackArray.begin(), heystackArray.end(), needleValue) != heystackArray.end(); + result = find(haystackArray.begin(), haystackArray.end(), needleValue) != haystackArray.end(); } return EvaluationResult(result); } @@ -98,19 +113,6 @@ ParseResult In::parse(const Convertible& value, ParsingContext& ctx) { return ParseResult(std::make_unique(std::move(*needle), std::move(*haystack))); } -bool In::isComparableType(type::Type type) { - return type == type::Boolean || type == type::String || type == type::Number || type == type::Null || - type == type::Value; -} - -bool In::isComparableRuntimeValue(type::Type type) { - return type == type::Boolean || type == type::String || type == type::Number || type == type::Null; -} - -bool In::isSearchableRuntimeValue(type::Type type) { - return type == type::String || type.is() || type == type::Null; -} - } // namespace expression } // namespace style } // namespace mbgl