reserve vecotr capacity, fix for points within polygon

Address review findings/nit

4 files changed, 46 insertions, 52 deletions

diff --git a/expression-test/expression_test_parser.cpp b/expression-test/expression_test_parser.cpp
index 0d414cc2c0..9edc3b9866 100644
--- a/expression-test/expression_test_parser.cpp
+++ b/expression-test/expression_test_parser.cpp
@@ -259,14 +259,13 @@ bool parseInputs(const JSValue& inputsValue, TestData& data) {
         // Parse canonicalID
         optional<CanonicalTileID> canonical;
         if (evaluationContext.HasMember("canonicalID")) {
-            assert(evaluationContext["canonicalID"].IsObject());
-            assert(
-                evaluationContext["canonicalID"].HasMember("z") && evaluationContext["canonicalID"]["z"].IsNumber() &&
-                evaluationContext["canonicalID"].HasMember("x") && evaluationContext["canonicalID"]["x"].IsNumber() &&
-                evaluationContext["canonicalID"].HasMember("y") && evaluationContext["canonicalID"]["y"].IsNumber());
-            canonical = CanonicalTileID(evaluationContext["canonicalID"]["z"].GetUint(),
-                                        evaluationContext["canonicalID"]["x"].GetUint(),
-                                        evaluationContext["canonicalID"]["y"].GetUint());
+            const auto& canonicalIDObject = evaluationContext["canonicalID"];
+            assert(canonicalIDObject.IsObject());
+            assert(canonicalIDObject.HasMember("z") && canonicalIDObject["z"].IsNumber());
+            assert(canonicalIDObject.HasMember("x") && canonicalIDObject["x"].IsNumber());
+            assert(canonicalIDObject.HasMember("y") && canonicalIDObject["y"].IsNumber());
+            canonical = CanonicalTileID(
+                canonicalIDObject["z"].GetUint(), canonicalIDObject["x"].GetUint(), canonicalIDObject["y"].GetUint());
         }
 
         // Parse availableImages
diff --git a/include/mbgl/style/expression/within.hpp b/include/mbgl/style/expression/within.hpp
index 5cc36ab19c..7e4050509d 100644
--- a/include/mbgl/style/expression/within.hpp
+++ b/include/mbgl/style/expression/within.hpp
@@ -15,7 +15,7 @@ class Within final : public Expression {
 public:
     explicit Within(GeoJSON geojson);
 
-    ~Within() final;
+    ~Within() override;
 
     EvaluationResult evaluate(const EvaluationContext&) const override;
 
diff --git a/src/mbgl/style/expression/within.cpp b/src/mbgl/style/expression/within.cpp
index a26f6fb7c8..0df0bd86ce 100644
--- a/src/mbgl/style/expression/within.cpp
+++ b/src/mbgl/style/expression/within.cpp
@@ -44,7 +44,7 @@ bool pointWithinPolygon(const mbgl::Point<double>& point, const mapbox::geometry
     for (auto ring : polygon) {
         auto size = ring.size();
         // loop through every edge of the ring
-        for (decltype(size) i = 0; i < size - 1; ++i) {
+        for (std::size_t i = 0; i < size - 1; ++i) {
             if (rayIntersect(point, ring[i], ring[i + 1])) {
                 within = !within;
             }
@@ -55,8 +55,7 @@ bool pointWithinPolygon(const mbgl::Point<double>& point, const mapbox::geometry
 
 bool pointWithinPolygons(const mbgl::Point<double>& point, const mapbox::geometry::multi_polygon<double>& polygons) {
     for (auto polygon : polygons) {
-        auto within = pointWithinPolygon(point, polygon);
-        if (within) return true;
+        if (pointWithinPolygon(point, polygon)) return true;
     }
     return false;
 }
@@ -77,14 +76,9 @@ bool pointsWithinPolygons(const mbgl::GeometryTileFeature& feature,
                                 return pointWithinPolygons(point, polygons);
                             },
                             [&polygons](const mapbox::geometry::multi_point<double>& points) -> bool {
-                                auto result = false;
-                                for (const auto& p : points) {
-                                    result = pointWithinPolygons(p, polygons);
-                                    if (!result) {
-                                        return result;
-                                    }
-                                }
-                                return result;
+                                return std::all_of(points.begin(), points.end(), [&polygons](const auto& p) {
+                                    return pointWithinPolygons(p, polygons);
+                                });
                             },
                             [](const auto&) -> bool { return false; });
                 },
@@ -95,14 +89,9 @@ bool pointsWithinPolygons(const mbgl::GeometryTileFeature& feature,
                                 return pointWithinPolygon(point, polygon);
                             },
                             [&polygon](const mapbox::geometry::multi_point<double>& points) -> bool {
-                                auto result = false;
-                                for (const auto& p : points) {
-                                    result = pointWithinPolygon(p, polygon);
-                                    if (!result) {
-                                        return result;
-                                    }
-                                }
-                                return result;
+                                return std::all_of(points.begin(), points.end(), [&polygon](const auto& p) {
+                                    return pointWithinPolygon(p, polygon);
+                                });
                             },
                             [](const auto&) -> bool { return false; });
                 },
@@ -138,7 +127,7 @@ namespace expression {
 
 Within::Within(GeoJSON geojson) : Expression(Kind::Within, type::Boolean), geoJSONSource(std::move(geojson)) {}
 
-Within::~Within() {}
+Within::~Within() = default;
 
 using namespace mbgl::style::conversion;
 
@@ -150,15 +139,15 @@ EvaluationResult Within::evaluate(const EvaluationContext& params) const {
     // Currently only support Point/Points in Polygon/Polygons
     if (geometryType == FeatureType::Point) {
         return pointsWithinPolygons(*params.feature, *params.canonical, geoJSONSource);
-    } else {
-        mbgl::Log::Warning(mbgl::Event::General, "within expression currently only support 'Point' geometry type");
     }
+    mbgl::Log::Warning(mbgl::Event::General, "within expression currently only support 'Point' geometry type");
+
     return false;
 }
 
 ParseResult Within::parse(const Convertible& value, ParsingContext& ctx) {
     if (isArray(value)) {
-        // object value, quoted with ["Within", value]
+        // object value, quoted with ["within", value]
         if (arrayLength(value) != 2) {
             ctx.error("'within' expression requires exactly one argument, but found " +
                       util::toString(arrayLength(value) - 1) + " instead.");
@@ -183,6 +172,7 @@ Value valueConverter(const mapbox::geojson::rapidjson_value& v) {
     }
     if (v.IsArray()) {
         std::vector<Value> result;
+        result.reserve(v.Size());
         for (const auto& m : v.GetArray()) {
             result.push_back(valueConverter(m));
         }
diff --git a/src/mbgl/tile/geometry_tile_data.cpp b/src/mbgl/tile/geometry_tile_data.cpp
index 56bef66c62..2f1c9e92fd 100644
--- a/src/mbgl/tile/geometry_tile_data.cpp
+++ b/src/mbgl/tile/geometry_tile_data.cpp
@@ -196,27 +196,27 @@ GeometryCollection convertGeometry(const Feature::geometry_type& geometryTileFea
     return geometryTileFeature.match(
         [&](const Point<double>& point) -> GeometryCollection { return {{latLonToTileCoodinates(point)}}; },
         [&](const MultiPoint<double>& points) -> GeometryCollection {
-            GeometryCollection result;
-            std::vector<Point<int16_t>> temp;
+            MultiPoint<int16_t> result;
+            result.reserve(points.size());
             for (const auto p : points) {
-                temp.emplace_back(latLonToTileCoodinates(p));
+                result.emplace_back(latLonToTileCoodinates(p));
             }
-            result = {temp};
-            return result;
+            return {std::move(result)};
         },
         [&](const LineString<double>& lineString) -> GeometryCollection {
-            GeometryCollection result;
-            std::vector<Point<int16_t>> temp;
+            LineString<int16_t> result;
+            result.reserve(lineString.size());
             for (const auto p : lineString) {
-                temp.emplace_back(latLonToTileCoodinates(p));
+                result.emplace_back(latLonToTileCoodinates(p));
             }
-            result = {temp};
-            return result;
+            return {std::move(result)};
         },
         [&](const MultiLineString<double>& lineStrings) -> GeometryCollection {
             GeometryCollection result;
+            result.reserve(lineStrings.size());
             for (const auto line : lineStrings) {
-                std::vector<Point<int16_t>> temp;
+                LineString<int16_t> temp;
+                temp.reserve(line.size());
                 for (const auto p : line) {
                     temp.emplace_back(latLonToTileCoodinates(p));
                 }
@@ -226,9 +226,11 @@ GeometryCollection convertGeometry(const Feature::geometry_type& geometryTileFea
         },
         [&](const Polygon<double> polygon) -> GeometryCollection {
             GeometryCollection result;
-            for (const auto line : polygon) {
-                std::vector<Point<int16_t>> temp;
-                for (const auto p : line) {
+            result.reserve(polygon.size());
+            for (const auto ring : polygon) {
+                LinearRing<int16_t> temp;
+                temp.reserve(ring.size());
+                for (const auto p : ring) {
                     temp.emplace_back(latLonToTileCoodinates(p));
                 }
                 result.emplace_back(temp);
@@ -237,14 +239,17 @@ GeometryCollection convertGeometry(const Feature::geometry_type& geometryTileFea
         },
         [&](const MultiPolygon<double> polygons) -> GeometryCollection {
             GeometryCollection result;
-            for (const auto polygon : polygons)
-                for (const auto line : polygon) {
-                    LineString<int16_t> temp;
-                    for (const auto p : line) {
-                        temp.emplace_back(latLonToTileCoodinates(p));
+            result.reserve(polygons.size());
+            for (const auto pg : polygons) {
+                for (const auto r : pg) {
+                    LinearRing<int16_t> ring;
+                    ring.reserve(r.size());
+                    for (const auto p : r) {
+                        ring.emplace_back(latLonToTileCoodinates(p));
                     }
-                    result.emplace_back(temp);
+                    result.emplace_back(ring);
                 }
+            }
             return result;
         },
         [](const auto&) -> GeometryCollection { return GeometryCollection(); });