Misc code cleanups.

* Use {} in constructors instead of ()
* Use "using" instead of "typedef"
* Use osmium::Tags::has_tag() instead of rolling our own
* Use const and noexcept in more places
* Add some asserts
* Formatting

Author: joto

src/coastline_handlers.hpp

@@ -48,17 +48,15 @@ class CoastlineHandlerPass1 : public osmium::handler::Handler {
 
     void way(const osmium::Way& way) {
         // We are only interested in ways tagged with natural=coastline.
-        const char* natural = way.tags().get_value_by_key("natural");
-        if (natural && !std::strcmp(natural, "coastline")) {
-            const char* bogus = way.tags().get_value_by_key("coastline");
-            if (bogus && !std::strcmp(bogus, "bogus")) {
+        if (way.tags().has_tag("natural", "coastline")) {
+            if (way.tags().has_tag("coastline", "bogus")) {
                 return; // ignore bogus coastline in Antarctica
             }
             m_coastline_rings.add_way(way);
         }
     }
 
-};
+}; // class CoastlineHandlerPass1
 
 /**
  * Osmium handler for the second pass over the input file in which
@@ -89,8 +87,7 @@ class CoastlineHandlerPass2 : public osmium::handler::Handler {
     }
 
     void node(const osmium::Node& node) {
-        const char* natural = node.tags().get_value_by_key("natural");
-        if (natural && !std::strcmp(natural, "coastline")) {
+        if (node.tags().has_tag("natural", "coastline")) {
             try {
                 m_output.add_error_point(m_factory.create_point(node), "tagged_node", node.id());
             } catch (const osmium::geometry_error&) {
@@ -104,6 +101,6 @@ class CoastlineHandlerPass2 : public osmium::handler::Handler {
         }
     }
 
-};
+}; // class CoastlineHandlerPass2
 
 #endif // COASTLINE_HANDLERS_HPP

src/coastline_polygons.cpp

@@ -46,14 +46,14 @@ std::unique_ptr<OGRPolygon> CoastlinePolygons::create_rectangular_polygon(double
     // make sure we are inside the bounds for the output SRS
     e.Intersect(srs.max_extent());
 
-    std::unique_ptr<OGRLinearRing> ring { new OGRLinearRing() };
+    std::unique_ptr<OGRLinearRing> ring{new OGRLinearRing()};
     ring->addPoint(e.MinX, e.MinY);
     ring->addPoint(e.MinX, e.MaxY);
     ring->addPoint(e.MaxX, e.MaxY);
     ring->addPoint(e.MaxX, e.MinY);
     ring->closeRings();
 
-    std::unique_ptr<OGRPolygon> polygon { new OGRPolygon() };
+    std::unique_ptr<OGRPolygon> polygon{new OGRPolygon()};
     polygon->addRingDirectly(ring.release());
     polygon->assignSpatialReference(srs.out());
 
@@ -95,7 +95,7 @@ void CoastlinePolygons::split_geometry(std::unique_ptr<OGRGeometry>&& geom, int
     } else { // wkbMultiPolygon
         const auto mp = static_cast_unique_ptr<OGRMultiPolygon>(std::move(geom));
         while (mp->getNumGeometries() > 0) {
-            std::unique_ptr<OGRPolygon> polygon { static_cast<OGRPolygon*>(mp->getGeometryRef(0)) };
+            std::unique_ptr<OGRPolygon> polygon{static_cast<OGRPolygon*>(mp->getGeometryRef(0))};
             mp->removeGeometry(0, false);
             polygon->assignSpatialReference(srs.out());
             split_polygon(std::move(polygon), level);
@@ -108,7 +108,7 @@ void CoastlinePolygons::split_polygon(std::unique_ptr<OGRPolygon>&& polygon, int
         m_max_split_depth = level;
     }
 
-    int num_points = polygon->getExteriorRing()->getNumPoints();
+    const int num_points = polygon->getExteriorRing()->getNumPoints();
     if (num_points <= m_max_points_in_polygon) {
         // do not split the polygon if it is small enough
         m_polygons.push_back(std::move(polygon));
@@ -139,7 +139,7 @@ void CoastlinePolygons::split_polygon(std::unique_ptr<OGRPolygon>&& polygon, int
             }
 
             // split vertically
-            double MidY = (envelope.MaxY+envelope.MinY) / 2;
+            const double MidY = (envelope.MaxY+envelope.MinY) / 2;
 
             b1 = create_rectangular_polygon(envelope.MinX, envelope.MinY, envelope.MaxX, MidY, m_expand);
             b2 = create_rectangular_polygon(envelope.MinX, MidY, envelope.MaxX, envelope.MaxY, m_expand);
@@ -151,15 +151,15 @@ void CoastlinePolygons::split_polygon(std::unique_ptr<OGRPolygon>&& polygon, int
             }
 
             // split horizontally
-            double MidX = (envelope.MaxX+envelope.MinX) / 2;
+            const double MidX = (envelope.MaxX+envelope.MinX) / 2;
 
             b1 = create_rectangular_polygon(envelope.MinX, envelope.MinY, MidX, envelope.MaxY, m_expand);
             b2 = create_rectangular_polygon(MidX, envelope.MinY, envelope.MaxX, envelope.MaxY, m_expand);
         }
 
         // Use intersection with bbox polygons to split polygon into two halfes
-        std::unique_ptr<OGRGeometry> geom1 { polygon->Intersection(b1.get()) };
-        std::unique_ptr<OGRGeometry> geom2 { polygon->Intersection(b2.get()) };
+        std::unique_ptr<OGRGeometry> geom1{polygon->Intersection(b1.get())};
+        std::unique_ptr<OGRGeometry> geom2{polygon->Intersection(b2.get())};
 
         if (geom1 && (geom1->getGeometryType() == wkbPolygon || geom1->getGeometryType() == wkbMultiPolygon) &&
             geom2 && (geom2->getGeometryType() == wkbPolygon || geom2->getGeometryType() == wkbMultiPolygon)) {
@@ -247,12 +247,12 @@ void CoastlinePolygons::add_line_to_output(std::unique_ptr<OGRLineString> line,
 // Add a coastline ring as LineString to output. Segments in this line that are
 // near the southern edge of the map or near the antimeridian are suppressed.
 void CoastlinePolygons::output_polygon_ring_as_lines(int max_points, const OGRLinearRing* ring) const {
-    int num = ring->getNumPoints();
+    const int num = ring->getNumPoints();
     assert(num > 2);
 
-    std::unique_ptr<OGRPoint> point1 { new OGRPoint };
-    std::unique_ptr<OGRPoint> point2 { new OGRPoint };
-    std::unique_ptr<OGRLineString> line { new OGRLineString };
+    std::unique_ptr<OGRPoint> point1{new OGRPoint};
+    std::unique_ptr<OGRPoint> point2{new OGRPoint};
+    std::unique_ptr<OGRLineString> line{new OGRLineString};
 
     ring->getPoint(0, point1.get());
     for (int i=1; i < num; ++i) {
@@ -262,7 +262,7 @@ void CoastlinePolygons::output_polygon_ring_as_lines(int max_points, const OGRLi
 
         if (line->getNumPoints() >= max_points || !added) {
             if (line->getNumPoints() >= 2) {
-                std::unique_ptr<OGRLineString> new_line { new OGRLineString };
+                std::unique_ptr<OGRLineString> new_line{new OGRLineString};
                 using std::swap;
                 swap(line, new_line);
                 add_line_to_output(std::move(new_line), ring->getSpatialReference());
@@ -333,7 +333,7 @@ void CoastlinePolygons::split_bbox(OGREnvelope e, polygon_vector_type&& v) {
 
         if (e.MaxX - e.MinX < e.MaxY-e.MinY) {
             // split vertically
-            double MidY = (e.MaxY+e.MinY) / 2;
+            const double MidY = (e.MaxY+e.MinY) / 2;
 
             e1.MinX = e.MinX;
             e1.MinY = e.MinY;
@@ -347,7 +347,7 @@ void CoastlinePolygons::split_bbox(OGREnvelope e, polygon_vector_type&& v) {
 
         } else {
             // split horizontally
-            double MidX = (e.MaxX+e.MinX) / 2;
+            const double MidX = (e.MaxX+e.MinX) / 2;
 
             e1.MinX = e.MinX;
             e1.MinY = e.MinY;
@@ -371,8 +371,8 @@ void CoastlinePolygons::split_bbox(OGREnvelope e, polygon_vector_type&& v) {
             OGREnvelope e;
             polygon->getEnvelope(&e);
 
-            bool e1_intersects_e = e1.Intersects(e);
-            bool e2_intersects_e = e2.Intersects(e);
+            const bool e1_intersects_e = e1.Intersects(e);
+            const bool e2_intersects_e = e2.Intersects(e);
 
             if (e1_intersects_e && e2_intersects_e) {
                 v1.emplace_back(static_cast<OGRPolygon*>(polygon->clone()));

src/coastline_polygons.hpp

@@ -31,7 +31,7 @@
 class OGRSpatialReference;
 class OutputDatabase;
 
-typedef std::vector<std::unique_ptr<OGRPolygon>> polygon_vector_type;
+using polygon_vector_type = std::vector<std::unique_ptr<OGRPolygon>>;
 
 /**
  * A collection of land polygons created out of coastlines.
@@ -120,6 +120,6 @@ class CoastlinePolygons {
     /// Write all coastlines to the output database (as lines).
     void output_lines(int max_points) const;
 
-};
+}; // class CoastlinePolygons
 
 #endif // COASTLINE_POLYGONS_HPP

src/coastline_ring.cpp

@@ -95,7 +95,7 @@ void CoastlineRing::close_ring() {
 }
 
 void CoastlineRing::close_antarctica_ring(int epsg) {
-    double min = epsg == 4326 ? -90.0 : -85.0511288;
+    const double min = epsg == 4326 ? -90.0 : -85.0511288;
 
     for (int lat = -78; lat > int(min); --lat) {
         m_way_node_list.emplace_back(0, osmium::Location(-179.99999, double(lat)));
@@ -137,18 +137,18 @@ std::unique_ptr<OGRLineString> CoastlineRing::ogr_linestring(osmium::geom::OGRFa
 
 std::unique_ptr<OGRPoint> CoastlineRing::ogr_first_point() const {
     const osmium::NodeRef& node_ref = m_way_node_list.front();
-    return std::unique_ptr<OGRPoint>(new OGRPoint(node_ref.lon(), node_ref.lat()));
+    return std::unique_ptr<OGRPoint>{new OGRPoint(node_ref.lon(), node_ref.lat())};
 }
 
 std::unique_ptr<OGRPoint> CoastlineRing::ogr_last_point() const {
     const osmium::NodeRef& node_ref = m_way_node_list.back();
-    return std::unique_ptr<OGRPoint>(new OGRPoint(node_ref.lon(), node_ref.lat()));
+    return std::unique_ptr<OGRPoint>{new OGRPoint(node_ref.lon(), node_ref.lat())};
 }
 
 // Pythagoras doesn't work on a round earth but that is ok here, we only need a
 // rough measure anyway
 double CoastlineRing::distance_to_start_position(osmium::Location pos) const {
-    osmium::Location p = m_way_node_list.front().location();
+    const osmium::Location p = m_way_node_list.front().location();
     return (pos.lon() - p.lon()) * (pos.lon() - p.lon()) + (pos.lat() - p.lat()) * (pos.lat() - p.lat());
 }
 

src/coastline_ring.hpp

@@ -22,6 +22,7 @@
 
 */
 
+#include <cassert>
 #include <map>
 #include <memory>
 #include <ostream>
@@ -35,7 +36,7 @@ class OGRPoint;
 class OGRLineString;
 class OGRPolygon;
 
-typedef std::multimap<osmium::object_id_type, osmium::Location*> posmap_type;
+using posmap_type = std::multimap<osmium::object_id_type, osmium::Location*>;
 
 /**
  * The CoastlineRing class models a (possibly unfinished) ring of
@@ -108,41 +109,45 @@ class CoastlineRing {
 
     /// ID of first node in the ring.
     osmium::object_id_type first_node_id() const {
+        assert(!m_way_node_list.empty());
         return m_way_node_list.front().ref();
     }
 
     /// ID of last node in the ring.
     osmium::object_id_type last_node_id() const {
+        assert(!m_way_node_list.empty());
         return m_way_node_list.back().ref();
     }
 
     /// Position of the first node in the ring.
     osmium::Location first_position() const {
+        assert(!m_way_node_list.empty());
         return m_way_node_list.front().location();
     }
 
     /// Position of the last node in the ring.
     osmium::Location last_position() const {
+        assert(!m_way_node_list.empty());
         return m_way_node_list.back().location();
     }
 
     /// Return ID of this ring (defined as smallest ID of the ways making up the ring).
-    osmium::object_id_type ring_id() const {
+    osmium::object_id_type ring_id() const noexcept {
         return m_ring_id;
     }
 
     /**
      * Set ring ID. The ring will only get the new ID if it is smaller than the
      * old one.
      */
-    void update_ring_id(osmium::object_id_type new_id) {
+    void update_ring_id(osmium::object_id_type new_id) noexcept {
         if (new_id < m_ring_id) {
             m_ring_id = new_id;
         }
     }
 
     /// Returns the number of ways making up this ring.
-    unsigned int nways() const {
+    unsigned int nways() const noexcept {
         return m_nways;
     }
 
@@ -157,7 +162,7 @@ class CoastlineRing {
     }
 
     /// Was this ring fixed because of missing/wrong OSM data?
-    bool is_fixed() const {
+    bool is_fixed() const noexcept {
         return m_fixed;
     }
 
@@ -170,6 +175,7 @@ class CoastlineRing {
      * method does this.
      */
     void fake_close() {
+        assert(!m_way_node_list.empty());
         m_way_node_list.back().set_ref(first_node_id());
     }
 
@@ -258,7 +264,7 @@ class CoastlineRing {
 
     friend std::ostream& operator<<(std::ostream& out, const CoastlineRing& cp);
 
-};
+}; // class CoastlineRing
 
 inline bool operator<(const CoastlineRing& lhs, const CoastlineRing& rhs) {
     return lhs.first_position() < rhs.first_position();