Round out ObjectPointerCollection implementation and test it.

Author: joto

include/osmium/object_pointer_collection.hpp

@@ -61,20 +61,24 @@ namespace osmium {
      *   osmium::memory::Buffer buffer = reader.read();
      *   osmium::apply(buffer, objects);
      *
+     * It is not possible to remove pointers from the collection except by
+     * clearing the whole collection.
+     *
      */
     class ObjectPointerCollection : public osmium::handler::Handler {
 
-        std::vector<osmium::OSMObject*> m_objects;
+        std::vector<osmium::OSMObject*> m_objects{};
 
     public:
 
         using iterator       = boost::indirect_iterator<std::vector<osmium::OSMObject*>::iterator, osmium::OSMObject>;
         using const_iterator = boost::indirect_iterator<std::vector<osmium::OSMObject*>::const_iterator, const osmium::OSMObject>;
 
-        ObjectPointerCollection() noexcept :
-            m_objects() {
-        }
+        ObjectPointerCollection() noexcept = default;
 
+        /**
+         * Add a pointer to an object to the collection.
+         */
         void osm_object(osmium::OSMObject& object) {
             m_objects.push_back(&object);
         }
@@ -87,20 +91,35 @@ namespace osmium {
             std::sort(m_objects.begin(), m_objects.end(), std::forward<TCompare>(compare));
         }
 
+        /// Is the collection empty?
+        bool empty() const noexcept {
+            return m_objects.empty();
+        }
+
+        /// Return size of the collection.
+        size_t size() const noexcept {
+            return m_objects.size();
+        }
+
+        /// Clear the collection,
+        void clear() {
+            m_objects.clear();
+        }
+
         iterator begin() {
-            return iterator { m_objects.begin() };
+            return iterator{m_objects.begin()};
         }
 
         iterator end() {
-            return iterator { m_objects.end() };
+            return iterator{m_objects.end()};
         }
 
         const_iterator cbegin() const {
-            return const_iterator { m_objects.cbegin() };
+            return const_iterator{m_objects.cbegin()};
         }
 
         const_iterator cend() const {
-            return const_iterator { m_objects.cend() };
+            return const_iterator{m_objects.cend()};
         }
 
     }; // class ObjectPointerCollection

include/osmium/osm/object_comparisons.hpp

@@ -33,6 +33,7 @@ DEALINGS IN THE SOFTWARE.
 
 */
 
+#include <cassert>
 #include <tuple>
 
 #include <osmium/osm/object.hpp>
@@ -51,7 +52,9 @@ namespace osmium {
             return lhs == rhs;
         }
 
+        /// @pre lhs and rhs must not be nullptr
         bool operator()(const osmium::OSMObject* lhs, const osmium::OSMObject* rhs) const noexcept {
+            assert(lhs && rhs);
             return *lhs == *rhs;
         }
 
@@ -68,7 +71,9 @@ namespace osmium {
                    lhs.id() == rhs.id();
         }
 
+        /// @pre lhs and rhs must not be nullptr
         bool operator()(const osmium::OSMObject* lhs, const osmium::OSMObject* rhs) const noexcept {
+            assert(lhs && rhs);
             return operator()(*lhs, *rhs);
         }
 
@@ -84,7 +89,9 @@ namespace osmium {
             return lhs < rhs;
         }
 
+        /// @pre lhs and rhs must not be nullptr
         bool operator()(const osmium::OSMObject* lhs, const osmium::OSMObject* rhs) const noexcept {
+            assert(lhs && rhs);
             return *lhs < *rhs;
         }
 
@@ -104,7 +111,9 @@ namespace osmium {
                    const_tie(rhs.type(), rhs.id() < 0, rhs.positive_id(), lhs.version(), lhs.timestamp());
         }
 
+        /// @pre lhs and rhs must not be nullptr
         bool operator()(const osmium::OSMObject* lhs, const osmium::OSMObject* rhs) const noexcept {
+            assert(lhs && rhs);
             return operator()(*lhs, *rhs);
         }
 

test/CMakeLists.txt

@@ -158,6 +158,7 @@ add_unit_test(geom test_wkt)
 add_unit_test(index test_id_set)
 add_unit_test(index test_id_to_location ENABLE_IF ${SPARSEHASH_FOUND})
 add_unit_test(index test_file_based_index)
+add_unit_test(index test_object_pointer_collection)
 
 add_unit_test(io test_compression_factory)
 add_unit_test(io test_bzip2 ENABLE_IF ${BZIP2_FOUND} LIBS ${BZIP2_LIBRARIES})

test/t/index/test_object_pointer_collection.cpp

@@ -0,0 +1,86 @@
+
+#include "catch.hpp"
+
+#include <osmium/builder/attr.hpp>
+#include <osmium/memory/buffer.hpp>
+#include <osmium/object_pointer_collection.hpp>
+#include <osmium/osm/object_comparisons.hpp>
+#include <osmium/visitor.hpp>
+
+using namespace osmium::builder::attr;
+
+TEST_CASE("Create ObjectPointerCollection") {
+    osmium::memory::Buffer buffer{1024, osmium::memory::Buffer::auto_grow::yes};
+
+    osmium::builder::add_node(buffer,
+        _id(3),
+        _version(3)
+    );
+
+    osmium::builder::add_node(buffer,
+        _id(1),
+        _version(2)
+    );
+
+    osmium::builder::add_node(buffer,
+        _id(1),
+        _version(4)
+    );
+
+    osmium::ObjectPointerCollection collection;
+    REQUIRE(collection.empty());
+    REQUIRE(collection.size() == 0);
+
+    osmium::apply(buffer, collection);
+
+    REQUIRE_FALSE(collection.empty());
+    REQUIRE(collection.size() == 3);
+
+    auto it = collection.cbegin();
+    REQUIRE(it->id() == 3);
+    REQUIRE(it->version() == 3);
+    ++it;
+    REQUIRE(it->id() == 1);
+    REQUIRE(it->version() == 2);
+    ++it;
+    REQUIRE(it->id() == 1);
+    REQUIRE(it->version() == 4);
+    ++it;
+    REQUIRE(it == collection.cend());
+
+    collection.sort(osmium::object_order_type_id_version{});
+
+    REQUIRE(collection.size() == 3);
+
+    it = collection.cbegin();
+    REQUIRE(it->id() == 1);
+    REQUIRE(it->version() == 2);
+    ++it;
+    REQUIRE(it->id() == 1);
+    REQUIRE(it->version() == 4);
+    ++it;
+    REQUIRE(it->id() == 3);
+    REQUIRE(it->version() == 3);
+    ++it;
+    REQUIRE(it == collection.cend());
+
+    collection.sort(osmium::object_order_type_id_reverse_version{});
+
+    it = collection.cbegin();
+    REQUIRE(it->id() == 1);
+    REQUIRE(it->version() == 4);
+    ++it;
+    REQUIRE(it->id() == 1);
+    REQUIRE(it->version() == 2);
+    ++it;
+    REQUIRE(it->id() == 3);
+    REQUIRE(it->version() == 3);
+    ++it;
+    REQUIRE(it == collection.cend());
+
+    collection.clear();
+
+    REQUIRE(collection.empty());
+    REQUIRE(collection.size() == 0);
+}
+