From 4935a2daf9c719ac4541e60b05d53ca486ef951d Mon Sep 17 00:00:00 2001
From: "Noah S. Elliott" <elliott22@llnl.gov>
Date: Wed, 9 Oct 2024 16:56:19 -0700
Subject: [PATCH 1/3] Move ItemCollection and derived classes to core

---
 src/axom/core/CMakeLists.txt                  |  4 +++
 .../{sidre => }/core/IndexedCollection.hpp    | 31 +++++++++---------
 src/axom/{sidre => }/core/ItemCollection.hpp  | 32 ++++++++-----------
 src/axom/core/IteratorBase.hpp                |  3 ++
 src/axom/{sidre => }/core/ListCollection.hpp  | 31 ++++++++++--------
 src/axom/{sidre => }/core/MapCollection.hpp   | 23 ++++++-------
 src/axom/core/Types.hpp                       |  2 ++
 src/axom/core/utilities/StringUtilities.hpp   |  9 ++++++
 src/axom/sidre/CMakeLists.txt                 |  4 ---
 src/axom/sidre/core/DataStore.cpp             |  4 +--
 src/axom/sidre/core/DataStore.hpp             | 16 +++-------
 src/axom/sidre/core/Group.cpp                 |  4 +--
 src/axom/sidre/core/Group.hpp                 |  8 ++---
 src/axom/sidre/core/SidreTypes.hpp            | 18 +++++++----
 src/axom/sidre/tests/sidre_collections.cpp    | 24 +++++++-------
 15 files changed, 110 insertions(+), 103 deletions(-)
 rename src/axom/{sidre => }/core/IndexedCollection.hpp (93%)
 rename src/axom/{sidre => }/core/ItemCollection.hpp (93%)
 rename src/axom/{sidre => }/core/ListCollection.hpp (92%)
 rename src/axom/{sidre => }/core/MapCollection.hpp (95%)

diff --git a/src/axom/core/CMakeLists.txt b/src/axom/core/CMakeLists.txt
index 39627d95ac..f1dbf22b26 100644
--- a/src/axom/core/CMakeLists.txt
+++ b/src/axom/core/CMakeLists.txt
@@ -56,9 +56,13 @@ set(core_headers
     ArrayIteratorBase.hpp
     ArrayView.hpp
     MDMapping.hpp
+    IndexedCollection.hpp
+    ItemCollection.hpp
     IteratorBase.hpp
+    ListCollection.hpp
     Macros.hpp
     Map.hpp
+    MapCollection.hpp
     FlatMap.hpp
     NumericLimits.hpp
     Path.hpp
diff --git a/src/axom/sidre/core/IndexedCollection.hpp b/src/axom/core/IndexedCollection.hpp
similarity index 93%
rename from src/axom/sidre/core/IndexedCollection.hpp
rename to src/axom/core/IndexedCollection.hpp
index c15cd1c66b..eb3908dbd7 100644
--- a/src/axom/sidre/core/IndexedCollection.hpp
+++ b/src/axom/core/IndexedCollection.hpp
@@ -7,6 +7,7 @@
 #define SIDRE_INDEXED_COLLECTION_HPP_
 
 // Standard C++ headers
+#include <iostream>
 #include <map>
 #include <stack>
 #include <string>
@@ -14,17 +15,12 @@
 
 // Other axom headers
 #include "axom/config.hpp"
-#include "axom/core/Types.hpp"
+#include "axom/core/ItemCollection.hpp"
 #include "axom/core/Macros.hpp"
-
-// Sidre project headers
-#include "SidreTypes.hpp"
-#include "ItemCollection.hpp"
+#include "axom/core/Types.hpp"
 
 namespace axom
 {
-namespace sidre
-{
 /*!
  *************************************************************************
  *
@@ -99,18 +95,18 @@ class IndexedCollection : public ItemCollection<T>
   /*!
    * \brief  Insert \a item at index \a idx if that index is not already occupied
    *
-   * \return Index at which \a item was inserted, if successful; sidre::InvalidIndex otherwise
+   * \return Index at which \a item was inserted, if successful; axom::InvalidIndex otherwise
    */
   IndexType insertItem(T* item, IndexType idx)
   {
     if(hasItem(idx))
     {
-      return sidre::InvalidIndex;
+      return axom::InvalidIndex;
     }
 
     if(idx < 0)
     {
-      return sidre::InvalidIndex;
+      return axom::InvalidIndex;
     }
 
     // grow capacity to support insertion at index
@@ -172,7 +168,7 @@ class IndexedCollection : public ItemCollection<T>
    */
   IndexType getValidEmptyIndex()
   {
-    IndexType newIndex = sidre::InvalidIndex;
+    IndexType newIndex = axom::InvalidIndex;
     bool found_empty_index = false;
 
     // try to find an empty index from the stack
@@ -195,9 +191,15 @@ class IndexedCollection : public ItemCollection<T>
       newIndex = m_items.size();
     }
 
-    SLIC_ASSERT_MSG(
-      isInClosedRange(newIndex) && !hasItem(newIndex),
-      "Index " << newIndex << " in IndexedCollection is not a valid empty index");
+#ifdef AXOM_DEBUG
+    if(!isInClosedRange(newIndex) || hasItem(newIndex))
+    {
+      std::cerr << "Index " << newIndex
+                << " in IndexedCollection is not a valid empty index"
+                << std::endl;
+    }
+    assert(isInClosedRange(newIndex) && !hasItem(newIndex));
+#endif
 
     return newIndex;
   }
@@ -283,7 +285,6 @@ T* IndexedCollection<T>::removeItem(IndexType idx)
   return nullptr;
 }
 
-}  // end namespace sidre
 }  // end namespace axom
 
 #endif  // SIDRE_INDEXED_COLLECTION_HPP_
diff --git a/src/axom/sidre/core/ItemCollection.hpp b/src/axom/core/ItemCollection.hpp
similarity index 93%
rename from src/axom/sidre/core/ItemCollection.hpp
rename to src/axom/core/ItemCollection.hpp
index be3eb3aa0a..4ab2d60b5c 100644
--- a/src/axom/sidre/core/ItemCollection.hpp
+++ b/src/axom/core/ItemCollection.hpp
@@ -12,7 +12,7 @@
  *
  *          This is a templated abstract base class defining an interface for
  *          classes holding a collection of items of a fixed
- *          type that can be accessed by string name or sidre::IndexType.
+ *          type that can be accessed by string name or axom::IndexType.
  *
  *          The primary intent is to decouple the implementation of the
  *          collections from the Group class which owns collections of
@@ -39,12 +39,12 @@
  *               size_t getNumItems() const;
  *
  *          - // Return first valid item index for iteration.
- *            // sidre::InvalidIndex returned if no items in collection
+ *            // axom::InvalidIndex returned if no items in collection
  *
  *               IndexType getFirstValidIndex() const;
  *
  *          - // Return next valid item index for iteration.
- *            // sidre::InvalidIndex returned if there are no more items
+ *            // axom::InvalidIndex returned if there are no more items
  *            // to be iterated over.
  *
  *               IndexType getNextValidIndex(IndexType idx) const;
@@ -68,12 +68,12 @@
  *               T const* getItem(IndexType idx) const;
  *
  *          - // Return name of object with given index
- *            // (sidre::InvalidName if none).
+ *            // (axom::utilities::string::InvalidName if none).
  *
  *               std::string getItemName(IndexType idx) const;
  *
  *          - // Return index of object with given name
- *            // (sidre::InvalidIndex if none).
+ *            // (axom::InvalidIndex if none).
  *
  *               IndexType getItemIndex(const std::string& name) const;
  *
@@ -105,21 +105,18 @@
  ******************************************************************************
  */
 
-#ifndef SIDRE_ITEMCOLLECTIONS_HPP_
-#define SIDRE_ITEMCOLLECTIONS_HPP_
+#ifndef AXOM_ITEMCOLLECTIONS_HPP_
+#define AXOM_ITEMCOLLECTIONS_HPP_
+
+#include <string>
 
 // Other axom headers
 #include "axom/config.hpp"
 #include "axom/core/Types.hpp"
 #include "axom/core/IteratorBase.hpp"
 
-// Sidre project headers
-#include "SidreTypes.hpp"
-
 namespace axom
 {
-namespace sidre
-{
 /*!
  *************************************************************************
  *
@@ -221,9 +218,9 @@ class ItemCollection<T>::iterator : public IteratorBase<iterator, IndexType>
 public:
   iterator(CollectionType* coll, bool is_first) : m_collection(coll)
   {
-    SLIC_ASSERT(coll != nullptr);
+    assert(coll != nullptr);
 
-    BaseType::m_pos = is_first ? coll->getFirstValidIndex() : sidre::InvalidIndex;
+    BaseType::m_pos = is_first ? coll->getFirstValidIndex() : axom::InvalidIndex;
   }
 
   IndexType index() const { return BaseType::m_pos; }
@@ -275,9 +272,9 @@ class ItemCollection<T>::const_iterator
 public:
   const_iterator(const CollectionType* coll, bool is_first) : m_collection(coll)
   {
-    SLIC_ASSERT(coll != nullptr);
+    assert(coll != nullptr);
 
-    BaseType::m_pos = is_first ? coll->getFirstValidIndex() : sidre::InvalidIndex;
+    BaseType::m_pos = is_first ? coll->getFirstValidIndex() : axom::InvalidIndex;
   }
 
   IndexType index() const { return BaseType::m_pos; }
@@ -364,7 +361,6 @@ class ItemCollection<T>::const_iterator_adaptor
   const CollectionType* m_collection {nullptr};
 };
 
-} /* end namespace sidre */
 } /* end namespace axom */
 
-#endif /* SIDRE_ITEMCOLLECTIONS_HPP_ */
+#endif /* AXOM_ITEMCOLLECTIONS_HPP_ */
diff --git a/src/axom/core/IteratorBase.hpp b/src/axom/core/IteratorBase.hpp
index 60c5901001..7bd5f39c62 100644
--- a/src/axom/core/IteratorBase.hpp
+++ b/src/axom/core/IteratorBase.hpp
@@ -12,6 +12,9 @@
 #ifndef AXOM_ITERBASE_HPP_
 #define AXOM_ITERBASE_HPP_
 
+#include "axom/config.hpp"
+#include "axom/core/Macros.hpp"
+
 namespace axom
 {
 /**
diff --git a/src/axom/sidre/core/ListCollection.hpp b/src/axom/core/ListCollection.hpp
similarity index 92%
rename from src/axom/sidre/core/ListCollection.hpp
rename to src/axom/core/ListCollection.hpp
index c4e5ca0825..65d797eae1 100644
--- a/src/axom/sidre/core/ListCollection.hpp
+++ b/src/axom/core/ListCollection.hpp
@@ -32,12 +32,12 @@
  *               size_t getNumItems() const;
  *
  *          - // Return first item index for iteration.
- *            // sidre::InvalidIndex returned if no items in collection
+ *            // axom::InvalidIndex returned if no items in collection
  *
  *               IndexType getFirstValidIndex() const;
  *
  *          - // Return next valid item index for iteration.
- *            // sidre::InvalidIndex returned if there are no further items
+ *            // axom::InvalidIndex returned if there are no further items
  *
  *               IndexType getNextValidIndex(IndexType idx) const;
  * *
@@ -75,10 +75,11 @@
  ******************************************************************************
  */
 
-#ifndef SIDRE_LISTCOLLECTIONS_HPP_
-#define SIDRE_LISTCOLLECTIONS_HPP_
+#ifndef AXOM_LISTCOLLECTIONS_HPP_
+#define AXOM_LISTCOLLECTIONS_HPP_
 
 // Standard C++ headers
+#include <iostream>
 #include <map>
 #include <list>
 #include <stack>
@@ -90,13 +91,10 @@
 #include "axom/core/Types.hpp"
 
 // Sidre project headers
-#include "SidreTypes.hpp"
-#include "ItemCollection.hpp"
+#include "axom/core/ItemCollection.hpp"
 
 namespace axom
 {
-namespace sidre
-{
 ////////////////////////////////////////////////////////////////////////
 //
 // ListCollection keeps an index constant for each item
@@ -224,10 +222,16 @@ IndexType ListCollection<T>::getNextValidIndex(IndexType idx) const
 template <typename T>
 IndexType ListCollection<T>::insertItem(T* item, const std::string& name)
 {
-  SLIC_WARNING_IF(!name.empty(),
-                  "Item " << name << " added to Group "
-                          << "which holds items in list format. "
-                          << "The name of this item will be ignored.");
+#ifdef AXOM_DEBUG
+  if(!name.empty())
+  {
+    std::cerr << "Item " << name << " added to Group "
+              << "which holds items in list format. "
+              << "The name of this item will be ignored." << std::endl;
+  }
+#else
+  AXOM_UNUSED_VAR(name);
+#endif
 
   bool use_recycled_index = false;
   IndexType idx = m_items.size();
@@ -273,7 +277,6 @@ T* ListCollection<T>::removeItem(IndexType idx)
   return ret_val;
 }
 
-} /* end namespace sidre */
 } /* end namespace axom */
 
-#endif /* SIDRE_LIST_COLLECTIONS_HPP_ */
+#endif /* AXOM_LIST_COLLECTIONS_HPP_ */
diff --git a/src/axom/sidre/core/MapCollection.hpp b/src/axom/core/MapCollection.hpp
similarity index 95%
rename from src/axom/sidre/core/MapCollection.hpp
rename to src/axom/core/MapCollection.hpp
index 99c0545ffa..27e89d0e71 100644
--- a/src/axom/sidre/core/MapCollection.hpp
+++ b/src/axom/core/MapCollection.hpp
@@ -12,7 +12,7 @@
  *
  *          MapCollection is an implemenation of ItemCollection to
  *          hold a collection of items of a fixed type that can be accessed
- *          accessed by string name or sidre::IndexType.
+ *          accessed by string name or axom::IndexType.
  *
  *          The primary intent is to decouple the implementation of the
  *          collection of times from the Group class which owns collections of
@@ -39,13 +39,13 @@
  *
  *          - // Return first valid item index (i.e., smallest index over
  *            // all items).
- *            // sidre::InvalidIndex returned if no items in collection
+ *            // axom::InvalidIndex returned if no items in collection
  *
  *               IndexType getFirstValidIndex() const;
  *
  *          - // Return next valid item index after given index (i.e., smallest
  *            // index over all indices larger than given one).
- *            // sidre::InvalidIndex returned
+ *            // axom::InvalidIndex returned
  *
  *               IndexType getNextValidIndex(IndexType idx) const;
  *
@@ -68,12 +68,12 @@
  *               T const* getItem(IndexType idx) const;
  *
  *          - // Return name of object with given index
- *            // (sidre::InvalidName if none).
+ *            // (axom::utilities::string::InvalidName if none).
  *
  *               std::string getItemName(IndexType idx) const;
  *
  *          - // Return index of object with given name
- *            // (sidre::InvalidIndex if none).
+ *            // (axom::InvalidIndex if none).
  *
  *               IndexType getItemIndex(const std::string& name) const;
  *
@@ -105,8 +105,8 @@
  ******************************************************************************
  */
 
-#ifndef SIDRE_MAP_COLLECTIONS_HPP_
-#define SIDRE_MAP_COLLECTIONS_HPP_
+#ifndef AXOM_MAP_COLLECTIONS_HPP_
+#define AXOM_MAP_COLLECTIONS_HPP_
 
 // Standard C++ headers
 #include <map>
@@ -117,9 +117,9 @@
 // Other axom headers
 #include "axom/config.hpp"
 #include "axom/core/Types.hpp"
+#include "axom/core/utilities/StringUtilities.hpp"
 
 // Sidre project headers
-#include "SidreTypes.hpp"
 #include "ItemCollection.hpp"
 
 #if defined(AXOM_USE_SPARSEHASH)
@@ -130,8 +130,6 @@
 
 namespace axom
 {
-namespace sidre
-{
 ////////////////////////////////////////////////////////////////////////
 //
 // MapCollection keeps an index constant for each item
@@ -222,7 +220,7 @@ class MapCollection : public ItemCollection<T>
   const std::string& getItemName(IndexType idx) const
   {
     return (hasItem(idx) ? m_items[static_cast<unsigned>(idx)]->getName()
-                         : InvalidName);
+                         : axom::utilities::string::InvalidName);
   }
 
   ///
@@ -394,7 +392,6 @@ T* MapCollection<T>::removeItem(IndexType idx)
   }
 }
 
-} /* end namespace sidre */
 } /* end namespace axom */
 
-#endif /* SIDRE_MAP_COLLECTIONS_HPP_ */
+#endif /* AXOM_MAP_COLLECTIONS_HPP_ */
diff --git a/src/axom/core/Types.hpp b/src/axom/core/Types.hpp
index 519cfd8e5a..535c852570 100644
--- a/src/axom/core/Types.hpp
+++ b/src/axom/core/Types.hpp
@@ -66,6 +66,8 @@ using IndexType = std::int64_t;
 using IndexType = std::int32_t;
 #endif
 
+static constexpr IndexType InvalidIndex = -1;
+
 #ifdef AXOM_USE_MPI
 
 // Note: MSVC complains about uninitialized static const integer class members,
diff --git a/src/axom/core/utilities/StringUtilities.hpp b/src/axom/core/utilities/StringUtilities.hpp
index 3f411fcd64..b94565bf9e 100644
--- a/src/axom/core/utilities/StringUtilities.hpp
+++ b/src/axom/core/utilities/StringUtilities.hpp
@@ -16,6 +16,15 @@ namespace utilities
 {
 namespace string
 {
+
+/*!
+ * \brief An invalid name string used in axom components
+ *
+ * This is an empty string that is available for axom components to compare
+ * as an invalid name.
+ */
+static const std::string InvalidName;
+
 /*!
  * \brief Tests whether a string ends with another string
  * https://stackoverflow.com/questions/20446201/how-to-check-if-string-ends-with-txt/20446257
diff --git a/src/axom/sidre/CMakeLists.txt b/src/axom/sidre/CMakeLists.txt
index fac6704b9f..ce1a72bda9 100644
--- a/src/axom/sidre/CMakeLists.txt
+++ b/src/axom/sidre/CMakeLists.txt
@@ -26,10 +26,6 @@ set(sidre_headers
     core/View.hpp
     core/Attribute.hpp
     core/AttrValues.hpp
-    core/ItemCollection.hpp
-    core/IndexedCollection.hpp
-    core/ListCollection.hpp
-    core/MapCollection.hpp
     core/SidreTypes.hpp
     core/SidreDataTypeIds.h )
 
diff --git a/src/axom/sidre/core/DataStore.cpp b/src/axom/sidre/core/DataStore.cpp
index e772cd8b4e..87ae4b993b 100644
--- a/src/axom/sidre/core/DataStore.cpp
+++ b/src/axom/sidre/core/DataStore.cpp
@@ -13,12 +13,12 @@
 #include "DataStore.hpp"
 
 // Other axom headers
+#include "axom/core/IndexedCollection.hpp"
+#include "axom/core/MapCollection.hpp"
 #include "axom/slic/interface/slic.hpp"
 #include "axom/slic/streams/GenericOutputStream.hpp"
 
 // Sidre component headers
-#include "IndexedCollection.hpp"
-#include "MapCollection.hpp"
 #include "Buffer.hpp"
 #include "Group.hpp"
 #include "Attribute.hpp"
diff --git a/src/axom/sidre/core/DataStore.hpp b/src/axom/sidre/core/DataStore.hpp
index 2baf4179ad..e3256b1055 100644
--- a/src/axom/sidre/core/DataStore.hpp
+++ b/src/axom/sidre/core/DataStore.hpp
@@ -22,16 +22,16 @@
 
 // Other axom headers
 #include "axom/config.hpp"
+#include "axom/core/ItemCollection.hpp"
+#include "axom/core/IndexedCollection.hpp"
 #include "axom/core/Macros.hpp"
+#include "axom/core/MapCollection.hpp"
 #include "axom/core/Types.hpp"
 #include "axom/slic/interface/slic.hpp"
 
 // Sidre project headers
 #include "Attribute.hpp"
 #include "SidreTypes.hpp"
-#include "ItemCollection.hpp"
-#include "IndexedCollection.hpp"
-#include "MapCollection.hpp"
 
 namespace axom
 {
@@ -40,12 +40,6 @@ namespace sidre
 class Buffer;
 class Group;
 
-template <typename TYPE>
-class IndexedCollection;
-
-template <typename TYPE>
-class MapCollection;
-
 /*!
  * \class DataStore
  *
@@ -60,8 +54,8 @@ class MapCollection;
 class DataStore
 {
 public:
-  using AttributeCollection = MapCollection<Attribute>;
-  using BufferCollection = IndexedCollection<Buffer>;
+  using AttributeCollection = axom::MapCollection<Attribute>;
+  using BufferCollection = axom::IndexedCollection<Buffer>;
 
 public:
   /*!
diff --git a/src/axom/sidre/core/Group.cpp b/src/axom/sidre/core/Group.cpp
index 8fbef37b13..f4314dfadc 100644
--- a/src/axom/sidre/core/Group.cpp
+++ b/src/axom/sidre/core/Group.cpp
@@ -12,12 +12,12 @@
   #include "conduit_relay_io_hdf5.hpp"
 #endif
 
+#include "axom/core/ListCollection.hpp"
 #include "axom/core/Macros.hpp"
+#include "axom/core/MapCollection.hpp"
 #include "axom/core/Path.hpp"
 
 // Sidre headers
-#include "ListCollection.hpp"
-#include "MapCollection.hpp"
 #include "Buffer.hpp"
 #include "DataStore.hpp"
 
diff --git a/src/axom/sidre/core/Group.hpp b/src/axom/sidre/core/Group.hpp
index a9383f5bef..10c3edb21b 100644
--- a/src/axom/sidre/core/Group.hpp
+++ b/src/axom/sidre/core/Group.hpp
@@ -18,7 +18,9 @@
 
 // axom headers
 #include "axom/config.hpp"
+#include "axom/core/ItemCollection.hpp"
 #include "axom/core/Macros.hpp"
+#include "axom/core/MapCollection.hpp"
 #include "axom/core/Types.hpp"
 #include "axom/slic.hpp"
 #include "axom/export/sidre.h"
@@ -40,19 +42,15 @@
 // Sidre headers
 #include "SidreTypes.hpp"
 #include "View.hpp"
-#include "ItemCollection.hpp"
 
 namespace axom
 {
+
 namespace sidre
 {
 class Buffer;
 class Group;
 class DataStore;
-template <typename TYPE>
-class ItemCollection;
-template <typename TYPE>
-class MapCollection;
 
 /*!
  * \class Group
diff --git a/src/axom/sidre/core/SidreTypes.hpp b/src/axom/sidre/core/SidreTypes.hpp
index 2bfa3c3c60..d5f472fdeb 100644
--- a/src/axom/sidre/core/SidreTypes.hpp
+++ b/src/axom/sidre/core/SidreTypes.hpp
@@ -16,6 +16,7 @@
 #include "SidreDataTypeIds.h"
 #include "conduit.hpp"
 #include "axom/core/Types.hpp"
+#include "axom/core/utilities/StringUtilities.hpp"
 
 namespace axom
 {
@@ -47,7 +48,12 @@ using IndexType = axom::IndexType;
 /*!
  * \brief Common invalid index identifier used in sidre.
  */
-const IndexType InvalidIndex = SIDRE_InvalidIndex;
+constexpr IndexType InvalidIndex = axom::InvalidIndex;
+
+/*!
+ * \brief Common invalid name string usde in sidre.
+ */
+const std::string InvalidName = axom::utilities::string::InvalidName;
 
 /*!
  * \brief Returns true if idx is valid, else false.
@@ -57,15 +63,13 @@ const IndexType InvalidIndex = SIDRE_InvalidIndex;
  */
 inline bool indexIsValid(IndexType idx) { return idx != InvalidIndex; }
 
-/*!
- * \brief Common invalid name (string) identifier used in sidre.
- */
-const std::string InvalidName;
-
 /*!
  * \brief Returns true if name is valid, else false.
  */
-inline bool nameIsValid(const std::string& name) { return name != InvalidName; }
+inline bool nameIsValid(const std::string& name)
+{
+  return name != axom::utilities::string::InvalidName;
+}
 
 /*!
  * \brief Enum that holds the numeric data type id options for sidre types.
diff --git a/src/axom/sidre/tests/sidre_collections.cpp b/src/axom/sidre/tests/sidre_collections.cpp
index 3a7f395d56..9d70ae2cd3 100644
--- a/src/axom/sidre/tests/sidre_collections.cpp
+++ b/src/axom/sidre/tests/sidre_collections.cpp
@@ -37,7 +37,7 @@ class ItemCollectionTest : public ::testing::Test
   using ValueType = typename CollectionType::value_type;
 
   static constexpr bool IsNameBased =
-    std::is_same<sidre::MapCollection<ValueType>, CollectionType>::value;
+    std::is_same<axom::MapCollection<ValueType>, CollectionType>::value;
 
 protected:
   void SetUp() override { m_coll = new CollectionType; }
@@ -93,16 +93,16 @@ class ItemCollectionTest : public ::testing::Test
   }
 
 protected:
-  sidre::ItemCollection<ValueType>* m_coll {nullptr};
+  axom::ItemCollection<ValueType>* m_coll {nullptr};
 };
 
 using MyTypes =
-  ::testing::Types<sidre::IndexedCollection<double>,
-                   sidre::ListCollection<double>,
-                   //sidre::MapCollection<double>, // note: invalid since double doesn't have required getName()
-                   sidre::IndexedCollection<NamedItem>,
-                   sidre::ListCollection<NamedItem>,
-                   sidre::MapCollection<NamedItem>>;
+  ::testing::Types<axom::IndexedCollection<double>,
+                   axom::ListCollection<double>,
+                   //axom::MapCollection<double>, // note: invalid since double doesn't have required getName()
+                   axom::IndexedCollection<NamedItem>,
+                   axom::ListCollection<NamedItem>,
+                   axom::MapCollection<NamedItem>>;
 TYPED_TEST_SUITE(ItemCollectionTest, MyTypes);
 
 TYPED_TEST(ItemCollectionTest, testEmpty)
@@ -442,11 +442,11 @@ TYPED_TEST(ItemCollectionTest, iterators)
 
 template <typename TheValueType>
 class MapCollectionTest
-  : public ItemCollectionTest<sidre::MapCollection<TheValueType>>
+  : public ItemCollectionTest<axom::MapCollection<TheValueType>>
 {
 public:
   using ValueType = TheValueType;
-  using MapCollectionType = sidre::MapCollection<TheValueType>;
+  using MapCollectionType = axom::MapCollection<TheValueType>;
   using ItemCollectionBase = ItemCollectionTest<MapCollectionType>;
 
 protected:
@@ -678,11 +678,11 @@ TYPED_TEST(MapCollectionTest, insertAlreadyPresent)
 
 template <typename TheValueType>
 class IndexedCollectionTest
-  : public ItemCollectionTest<sidre::IndexedCollection<TheValueType>>
+  : public ItemCollectionTest<axom::IndexedCollection<TheValueType>>
 {
 public:
   using ValueType = TheValueType;
-  using IndexedCollectionType = sidre::IndexedCollection<TheValueType>;
+  using IndexedCollectionType = axom::IndexedCollection<TheValueType>;
   using ItemCollectionBase = ItemCollectionTest<IndexedCollectionType>;
 
 protected:

From bf2da57e72dc78877dd1fa70a7ec7b8526350363 Mon Sep 17 00:00:00 2001
From: "Noah S. Elliott" <elliott22@llnl.gov>
Date: Wed, 30 Oct 2024 10:49:40 -0700
Subject: [PATCH 2/3] Address some review comments

---
 RELEASE-NOTES.md                 |  3 +++
 src/axom/core/ItemCollection.hpp | 28 ----------------------------
 src/axom/core/MapCollection.hpp  |  2 +-
 3 files changed, 4 insertions(+), 29 deletions(-)

diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
index 05127e7540..28456fa045 100644
--- a/RELEASE-NOTES.md
+++ b/RELEASE-NOTES.md
@@ -22,6 +22,9 @@ The Axom project release numbers follow [Semantic Versioning](http://semver.org/
 ###  Added
 
 ###  Changed
+- ItemCollection and its child classes MapCollection, ListCollection, and IndexedCollection were moved from Sidre
+  to core.  The namespace prefix for these classes is now axom:: insteand of axom::sidre.  The internal usage of
+  these types within Sidre Datastore and Group is unchanged.
 
 ###  Deprecated
 
diff --git a/src/axom/core/ItemCollection.hpp b/src/axom/core/ItemCollection.hpp
index 4ab2d60b5c..e6bb7f2133 100644
--- a/src/axom/core/ItemCollection.hpp
+++ b/src/axom/core/ItemCollection.hpp
@@ -49,44 +49,20 @@
  *
  *               IndexType getNextValidIndex(IndexType idx) const;
  *
- *          - // Return true if item with given name in collection; else false.
- *
- *               bool hasItem(const std::string& name) const;
- *
  *          - // Return true if item with given index in collection; else false.
  *
  *               bool hasItem(IndexType idx) const;
  *
- *          - // Return pointer to item with given name (nullptr if none).
- *
- *               T* getItem(const std::string& name);
- *               T const* getItem(const std::string& name) const ;
- *
  *          - // Return pointer to item with given index (nullptr if none).
  *
  *               T* getItem(IndexType idx);
  *               T const* getItem(IndexType idx) const;
  *
- *          - // Return name of object with given index
- *            // (axom::utilities::string::InvalidName if none).
- *
- *               std::string getItemName(IndexType idx) const;
- *
- *          - // Return index of object with given name
- *            // (axom::InvalidIndex if none).
- *
- *               IndexType getItemIndex(const std::string& name) const;
- *
  *          - // Insert item with given name; return index if insertion
  *            // succeeded, and InvalidIndex otherwise.
  *
  *               IndexType insertItem(T* item, const std::string& name);
  *
- *          - // Remove item with given name if it exists and return a
- *            // pointer to it. If it doesn't exist, return nullptr.
- *
- *               T* removeItem(const std::string& name);
- *
  *          - // Remove item with given index if it exists and return a
  *            // pointer to it. If it doesn't exist, return nullptr.
  *
@@ -96,10 +72,6 @@
  *
  *               void removeAllItems();
  *
- *          - // Clear all items and destroy them.
- *
- *               void deleteAllItems();
- *
  *          \endverbatim
  *
  ******************************************************************************
diff --git a/src/axom/core/MapCollection.hpp b/src/axom/core/MapCollection.hpp
index 27e89d0e71..9c600d9760 100644
--- a/src/axom/core/MapCollection.hpp
+++ b/src/axom/core/MapCollection.hpp
@@ -70,7 +70,7 @@
  *          - // Return name of object with given index
  *            // (axom::utilities::string::InvalidName if none).
  *
- *               std::string getItemName(IndexType idx) const;
+ *               const std::string& getItemName(IndexType idx) const;
  *
  *          - // Return index of object with given name
  *            // (axom::InvalidIndex if none).

From d440e49c121f9e1a3f7f8623534823a80e5ee450 Mon Sep 17 00:00:00 2001
From: "Noah S. Elliott" <elliott22@llnl.gov>
Date: Wed, 30 Oct 2024 13:28:31 -0700
Subject: [PATCH 3/3] Move collections test to core

---
 src/axom/core/tests/CMakeLists.txt             | 16 ++++++++++++++++
 .../tests/core_collections.cpp}                | 18 +++++++-----------
 src/axom/sidre/tests/CMakeLists.txt            |  1 -
 3 files changed, 23 insertions(+), 12 deletions(-)
 rename src/axom/{sidre/tests/sidre_collections.cpp => core/tests/core_collections.cpp} (98%)

diff --git a/src/axom/core/tests/CMakeLists.txt b/src/axom/core/tests/CMakeLists.txt
index d1ebe2810d..25ea2444a1 100644
--- a/src/axom/core/tests/CMakeLists.txt
+++ b/src/axom/core/tests/CMakeLists.txt
@@ -79,6 +79,22 @@ foreach(test_suite ${core_serial_tests})
                    COMMAND core_serial_tests --gtest_filter=${test_suite}* )
 endforeach()
 
+#------------------------------------------------------------------------------
+# Test for ItemCollection classes
+#------------------------------------------------------------------------------
+
+axom_add_executable(NAME core_collections_test
+                    SOURCES core_collections.cpp
+                    OUTPUT_DIR ${TEST_OUTPUT_DIRECTORY}
+                    DEPENDS_ON ${core_test_depends}
+                    FOLDER axom/core/tests
+                    )
+
+axom_add_test( NAME    core_collections
+               COMMAND core_collections_test
+             )
+
+
 #------------------------------------------------------------------------------
 # MPI GTests
 #------------------------------------------------------------------------------
diff --git a/src/axom/sidre/tests/sidre_collections.cpp b/src/axom/core/tests/core_collections.cpp
similarity index 98%
rename from src/axom/sidre/tests/sidre_collections.cpp
rename to src/axom/core/tests/core_collections.cpp
index 9d70ae2cd3..fcafd3b9c2 100644
--- a/src/axom/sidre/tests/sidre_collections.cpp
+++ b/src/axom/core/tests/core_collections.cpp
@@ -3,20 +3,16 @@
 //
 // SPDX-License-Identifier: (BSD-3-Clause)
 
-/// Tests sidre's ItemCollection hierachy,
+/// Tests axom's ItemCollection hierachy,
 /// including MapCollection, ListCollection and IndexedCollection
 
 #include "gtest/gtest.h"
 
 #include "axom/config.hpp"
 #include "axom/core.hpp"
-#include "axom/slic.hpp"
-#include "axom/sidre.hpp"
 
 #include "axom/fmt.hpp"
 
-namespace sidre = axom::sidre;
-
 struct NamedItem
 {
   NamedItem(const std::string& name) : m_name(name) { }
@@ -45,7 +41,7 @@ class ItemCollectionTest : public ::testing::Test
   void TearDown() override
   {
     // Remove and deallocate items
-    for(auto idx = m_coll->getFirstValidIndex(); idx != sidre::InvalidIndex;
+    for(auto idx = m_coll->getFirstValidIndex(); idx != axom::InvalidIndex;
         idx = m_coll->getNextValidIndex(idx))
     {
       auto* val = m_coll->removeItem(idx);
@@ -655,14 +651,14 @@ TYPED_TEST(MapCollectionTest, insertAlreadyPresent)
       if(hasItem)
       {
         // new item was not removed, must deallocate
-        EXPECT_EQ(sidre::InvalidIndex, idx);
+        EXPECT_EQ(axom::InvalidIndex, idx);
         delete val;
         val = nullptr;
       }
       else
       {
         // new item was added
-        EXPECT_NE(sidre::InvalidIndex, idx);
+        EXPECT_NE(axom::InvalidIndex, idx);
         ++num_added;
       }
       EXPECT_TRUE(map_coll->hasItem(str));
@@ -874,14 +870,14 @@ TYPED_TEST(IndexedCollectionTest, insertAlreadyPresent)
       if(hasItem)
       {
         // new item was not removed, must deallocate
-        EXPECT_EQ(sidre::InvalidIndex, newIndex);
+        EXPECT_EQ(axom::InvalidIndex, newIndex);
         delete val;
         val = nullptr;
       }
       else
       {
         // new item was added
-        EXPECT_NE(sidre::InvalidIndex, newIndex);
+        EXPECT_NE(axom::InvalidIndex, newIndex);
         ++num_added;
       }
       EXPECT_TRUE(indexed_coll->hasItem(idx));
@@ -933,7 +929,7 @@ TYPED_TEST(IndexedCollectionTest, insertBadIdx)
       auto insertedIdx = indexed_coll->insertItem(val, idx);
       if(idx < 0)
       {
-        EXPECT_EQ(sidre::InvalidIndex, insertedIdx);
+        EXPECT_EQ(axom::InvalidIndex, insertedIdx);
         delete val;
       }
       else
diff --git a/src/axom/sidre/tests/CMakeLists.txt b/src/axom/sidre/tests/CMakeLists.txt
index e7735b9951..f264f36589 100644
--- a/src/axom/sidre/tests/CMakeLists.txt
+++ b/src/axom/sidre/tests/CMakeLists.txt
@@ -15,7 +15,6 @@ set(gtest_sidre_tests
    sidre_buffer.cpp
    sidre_buffer_unit.cpp
    sidre_class.cpp
-   sidre_collections.cpp
    sidre_datastore.cpp
    sidre_datastore_unit.cpp
    sidre_external.cpp