Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move ItemCollection and derived classes to core #1443

Merged
merged 8 commits into from
Oct 30, 2024
4 changes: 4 additions & 0 deletions src/axom/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,20 @@
#define SIDRE_INDEXED_COLLECTION_HPP_

// Standard C++ headers
#include <iostream>
#include <map>
#include <stack>
#include <string>
#include <vector>

// 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
{
/*!
*************************************************************************
*
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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;
}
Expand Down Expand Up @@ -283,7 +285,6 @@ T* IndexedCollection<T>::removeItem(IndexType idx)
return nullptr;
}

} // end namespace sidre
} // end namespace axom

#endif // SIDRE_INDEXED_COLLECTION_HPP_
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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;
*
Expand Down Expand Up @@ -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
{
/*!
*************************************************************************
*
Expand Down Expand Up @@ -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; }
Expand Down Expand Up @@ -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; }
Expand Down Expand Up @@ -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_ */
3 changes: 3 additions & 0 deletions src/axom/core/IteratorBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
#ifndef AXOM_ITERBASE_HPP_
#define AXOM_ITERBASE_HPP_

#include "axom/config.hpp"
#include "axom/core/Macros.hpp"

namespace axom
{
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
* *
Expand Down Expand Up @@ -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>
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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_ */
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
*
Expand All @@ -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;
*
Expand Down Expand Up @@ -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>
Expand All @@ -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)
Expand All @@ -130,8 +130,6 @@

namespace axom
{
namespace sidre
{
////////////////////////////////////////////////////////////////////////
//
// MapCollection keeps an index constant for each item
Expand Down Expand Up @@ -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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nselliott -- This function requires the templated type T to have a getName() function.

Could you please document this requirement?
Alternatively, is there a way to conditionally include this function via enable_if ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requirement is documented in the header. I think it's better not to add a SFINAE implementation, as the core feature of MapCollection is to hold a collection of named items. Using enable_if would make the code more complicated and allow instances of MapCollection to compile that don't have a meaningful use case, so in my opinion it's better to catch attempts to do that at compile time.

: InvalidName);
: axom::utilities::string::InvalidName);
}

///
Expand Down Expand Up @@ -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_ */
2 changes: 2 additions & 0 deletions src/axom/core/Types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions src/axom/core/utilities/StringUtilities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading