From 48a4bf6702c081288853a642749459ac5e42d6d4 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 8 Sep 2023 12:58:40 +0200 Subject: [PATCH] Improve the collection interface (#478) * Add empty method to collection interface * Add operator== do collection iterators * Fix clang-tidy warning --- include/podio/CollectionBase.h | 3 +++ include/podio/UserDataCollection.h | 5 +++++ python/templates/Collection.cc.jinja2 | 4 ++++ python/templates/Collection.h.jinja2 | 3 +++ python/templates/macros/iterator.jinja2 | 4 ++++ tests/unittests/frame.cpp | 2 +- tests/unittests/unittest.cpp | 15 ++++++++++----- 7 files changed, 30 insertions(+), 6 deletions(-) diff --git a/include/podio/CollectionBase.h b/include/podio/CollectionBase.h index 6e5b8ded4..ae6fa1d4a 100644 --- a/include/podio/CollectionBase.h +++ b/include/podio/CollectionBase.h @@ -55,6 +55,9 @@ class CollectionBase { /// number of elements in the collection virtual size_t size() const = 0; + /// Is the collection empty + virtual bool empty() const = 0; + /// fully qualified type name virtual const std::string_view getTypeName() const = 0; /// fully qualified type name of elements - with namespace diff --git a/include/podio/UserDataCollection.h b/include/podio/UserDataCollection.h index cc5b7154f..5093816ad 100644 --- a/include/podio/UserDataCollection.h +++ b/include/podio/UserDataCollection.h @@ -136,6 +136,11 @@ class UserDataCollection : public CollectionBase { return _vec.size(); } + /// Is the collection empty + bool empty() const override { + return _vec.empty(); + } + /// fully qualified type name const std::string_view getTypeName() const override { return typeName; diff --git a/python/templates/Collection.cc.jinja2 b/python/templates/Collection.cc.jinja2 index 44b4c139d..b7c49cdab 100644 --- a/python/templates/Collection.cc.jinja2 +++ b/python/templates/Collection.cc.jinja2 @@ -51,6 +51,10 @@ std::size_t {{ collection_type }}::size() const { return m_storage.entries.size(); } +bool {{ collection_type }}::empty() const { + return m_storage.entries.empty(); +} + void {{ collection_type }}::setSubsetCollection(bool setSubset) { if (m_isSubsetColl != setSubset && !m_storage.entries.empty()) { throw std::logic_error("Cannot change the character of a collection that already contains elements"); diff --git a/python/templates/Collection.h.jinja2 b/python/templates/Collection.h.jinja2 index cda71791e..0e49aec53 100644 --- a/python/templates/Collection.h.jinja2 +++ b/python/templates/Collection.h.jinja2 @@ -86,6 +86,9 @@ public: /// number of elements in the collection std::size_t size() const final; + /// Is the collection empty + bool empty() const final; + /// fully qualified type name const std::string_view getTypeName() const final { return typeName; } /// fully qualified type name of elements - with namespace diff --git a/python/templates/macros/iterator.jinja2 b/python/templates/macros/iterator.jinja2 index eaba99ba8..4485504bd 100644 --- a/python/templates/macros/iterator.jinja2 +++ b/python/templates/macros/iterator.jinja2 @@ -11,6 +11,10 @@ public: return m_index != x.m_index; // TODO: may not be complete } + bool operator==(const {{ iterator_type }}& x) const { + return m_index == x.m_index; // TODO: may not be complete + } + {{ prefix }}{{ class.bare_type }} operator*(); {{ prefix }}{{ class.bare_type }}* operator->(); {{ iterator_type }}& operator++(); diff --git a/tests/unittests/frame.cpp b/tests/unittests/frame.cpp index e52965a07..e76803fd7 100644 --- a/tests/unittests/frame.cpp +++ b/tests/unittests/frame.cpp @@ -220,7 +220,7 @@ void checkFrame(const podio::Frame& frame) { REQUIRE(hits[1].energy() == 1); REQUIRE(hits[1].cellID() == 0x123ULL); - REQUIRE(frame.get("emptyClusters").size() == 0); + REQUIRE(frame.get("emptyClusters").empty()); auto& clusters = frame.get("clusters"); REQUIRE(clusters.size() == 2); diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index cc87410f7..8a313de28 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -80,7 +80,6 @@ TEST_CASE("Assignment-operator ref count", "[basics][memory-management]") { } TEST_CASE("Clearing", "[UBSAN-FAIL][ASAN-FAIL][THREAD-FAIL][basics][memory-management]") { - bool success = true; auto store = podio::EventStore(); auto& hits = store.create("hits"); auto& clusters = store.create("clusters"); @@ -102,10 +101,7 @@ TEST_CASE("Clearing", "[UBSAN-FAIL][ASAN-FAIL][THREAD-FAIL][basics][memory-manag oneRels.push_back(oneRel); } hits.clear(); - if (hits.size() != 0) { - success = false; - } - REQUIRE(success); + REQUIRE(hits.empty()); } TEST_CASE("Cloning", "[basics][memory-management]") { @@ -440,6 +436,15 @@ TEST_CASE("NonPresentCollection", "[basics][event-store]") { REQUIRE_THROWS_AS(store.get("NonPresentCollection"), std::runtime_error); } +TEST_CASE("Collection size and empty", "[basics][collections]") { + ExampleClusterCollection coll{}; + REQUIRE(coll.empty()); + + coll.create(); + coll.create(); + REQUIRE(coll.size() == 2u); +} + TEST_CASE("const correct indexed access to const collections", "[const-correctness]") { STATIC_REQUIRE(std::is_same_v()[0]), ExampleCluster>); // const collections should only have indexed access to mutable