Creating an enumerator based on an ATL collection

Question

I am designing a COM wrapper for a C++ library using ATL. Currently I am a little bit confused about enumerators that are based on ATL collections. I already wrote several enumerators based on STL collections, but since there is no ICollectionOnATL-interface (such as the ICollectionOnSTL-interface), I am a little bit confused if my implementation is correct.

Here is how I am creating the enumerator at the moment:

STDMETHODIMP CCollectionOnATL::get_NewEnum(IEnumITEM** enumerator)
{
    // IItem implements IUnknown.
    typedef CInterfaceArray<IItem> Container;
    typedef CComEnum<IEnumITEM, &IID_IEnumITEM, IItem*, _CopyInterface<IItem>> CComEnumITEM;

    // Copy all elements to an array that can be enumerated.
    Container items; 
    auto elements = m_coll.GetCount();
    //items.SetCount(elements);

    // See below for the definition of m_coll.
    POSITION pos = m_coll.GetHeadPosition();

    for (size_t element = 0; element < elements; element++)
    {
        auto current = m_coll.GetNext(pos);

        // Automatically calls AddRef!
        items.Add(current->m_value);
    }

    // Create an enumerator over the array.
    // Using `new` is valid here (since enumerators do not implement FinalConstruct).
    CComObject<CComEnumITEM>* pEnum = new CComObject<CComEnumITEM>();
    pEnum->Init(&items[0], &items[elements - 1], nullptr, ATL::AtlFlagTakeOwnership);

    // Return the enumerator.
    return pEnum->QueryInterface(enumerator);
}

The member m_coll is defined as CRBMap:

CRBMap<ULONG, IItem*> m_coll;

There are some points with this implementation, where I am not really sure if I understood them correctly:

1. Iterating the CRBMap should give me an ordered list, where the lowest key is the first and the highest key is the last element. However, I am not sure if there's a "better" (in terms of simplicity) way to perform the iteration.

2. Usage of CInterfaceArray to initialize the enumerator:

   pEnum->Init(&items[0], &items[elements - 1], nullptr, ATL::AtlFlagTakeOwnership);
   

   Is the initialization correct? Especially I am wondering about the third parameter, that references an IUnknown-instance to keep alive for as long as the enumerator lives. Currently I am passing nullptr, but I think the right value should be the items-array, which however does not implement IUnknown.

3. Regarding the enumerator initialization: Was my choose of ATL::AtlFlagTakeOwnership correct in here? I am not completely sure, because the actual ownership of the instances is taken by m_coll.

4. Since the size of the final array is known, I could initialize it (and therefor prevent relocations/copy-steps) using items.SetCount(elements);. However, the MSDN only states that CInterfaceArray::Add automatically calls AddRef. There's no information about SetAt or operator[]. Could I simply change the way I add items to the array to

   items[element] = current->m_value; 
   

   without any change in the behaviour of reference counting?

Also if you have any suggestions on how I can further improve this code, please let me know!

Answer 1

Okay, I finally got this working. Here are my results after hours of trying to get this right:

1. Alternatively it is possible to use a while-loop, which feels somehow more natural, a little bit like a foreach loop:

   POSITION pos = m_coll.GetStartPosition();
   
   while (pos != nullptr)
   {
       IItem* entry = m_coll.GetNextValue(pos);
       entry->get_Address(&address);
   }
   

   Speaking of foreach, I wrote a small macro that makes it possible to actually write it like a foreach-loop:

   #define FOREACH(T, entry, collection) \
       POSITION pos = collection.GetHeadPosition(); \
       T entry; \
       while (pos != nullptr && (entry = collection.GetNextValue(pos)) != nullptr) 
   

   Now a loop can be written like this:

   FOREACH(IItem*, item, m_coll)
   {
       item->FooBar();
   }
   

   However this only works for CRBMaps, for CAtlMap you have to change the macro to use GetStartPosition() instead (Hooray consistent design!). For other collections this needs to be changed entirely. Also for initializing the enumerator is easier with the traditional for-based apprach, as shown below.

2. Turns out that the best way appears to be using a traditional old-fashioned array, because that's the fastest and simplest way to store data inside the enumerator. The initialization then receives a pointer to the first and a pointer to the first element after the last one of the array. The third parameter is used to keep the host of the enumerator (typically the creator/this) alive. If IUnknown is ambigouos here, use this->GetUnknown();.

3. The flag was chosen right in this scenario, because the array storing a snapshot of entries that can be iterated is out of control after the enumerator has been created and therefor also needs to released by the enumerator.

4. That's another plus for an traditional array.

Here's the complete code:

STDMETHODIMP CCollectionOnATL::get_NewEnum(IEnumITEM** enumerator)
{
    // Define the type of the enumerator.
    typedef CComEnum<IEnumITEM, &IID_IEnumITEM, IItem*, _CopyInterface<IItem>> IItemEnumerator;

    // Copy all elements to an array that can be iterated.
    size_t elements = m_coll.GetCount();
    IItem** items = new IItem*[elements];
    POSITION pos = m_coll.GetHeadPosition();

    for (size_t element = 0; element < elements; element++)
    {
        IItem* item = m_coll.GetNextValue(pos);

        // Add a reference of the current item to the enumeration.
        item->AddRef();
        items[element] = item;
    }

    // Create an enumerator for the array.
    CComObject<IItemEnumerator>* pEnum = new CComObject<IItemEnumerator>();
    HRESULT hr;

    if (FAILED(hr = pEnum->Init(&items[0], &items[elements], this->GetUnknown(), ATL::AtlFlagTakeOwnership)))
        return hr;

    return pEnum->QueryInterface(enumerator);
}