c++iteratorkey-valueavl-treepre-increment

Why is the pre-increment operator overload not being called for my custom iterator class in C++?


I'm trying to implement a custom map container class that uses an AVL tree data structure. I have all the basic functionality in place, and it all works fine. But when I try to create the necessary overloaded functions to make my container class compatible with C++ ranged-for loops, it seems like my iterator class's pre-increment operator overload (operator++) isn't being called.

I have a Map<key_type, value_type> template class, which is the actual container itself, and a Pair<key_type, value_type> template class, which is just a convenient data structure that holds the key-value pairs. The Pair class also doubles as an iterator for the Map.

In short, I'm trying to mimic the behaviour of Unreal Engine's TMap and TPair:

Iteration over TMaps is similar to TArrays. You can use the C++ ranged-for feature, remembering that the element type is a TPair.

Example code below:

TMap<int32, FString> FruitMap;
for (const TPair<int32, FString>& Fruit : FruitMap)
{
    FPlatformMisc::LocalPrint(*FString::Printf(TEXT("(%d, \"%s\")\n"), Fruit.Key, *Fruit.Value));
}

Here's my implementation for the Pair and Map classes:

template<typename key_type, typename value_type>
class Pair
{
    friend Map<key_type, value_type>;

public:
    Pair(const key_type& key, const value_type& value) :
        m_Key(key),
        m_Value(value)
    {

    }

private:
    Pair& operator++()
    {
        return (m_OwningMap != nullptr) ? m_OwningMap->GetNextPair(*this) : *this;
    }

public:
    key_type m_Key;
    value_type m_Value;

private:
    mutable Map<key_type, value_type>* m_OwningMap = nullptr;
};
template<typename key_type, typename value_type>
class Map // Ordered Map
{
    friend Pair<key_type, value_type>;

private:
    struct Node
    {
        Pair<key_type, value_type> m_Pair;
        size_t m_Depth;
        
        Node* m_ParentNode;
        Node* m_LeftChildNode;
        Node* m_RightChildNode;
    };

public:
    Map();
    Map(const Map<key_type, value_type>& otherMap);
    ~Map();

    Pair<key_type, value_type>* begin()
    {
        if (m_RootNode != nullptr)
        {
            Node* leftMostChildNode = m_RootNode;
            while (leftMostChildNode->m_LeftChildNode != nullptr)
            {
                leftMostChildNode = leftMostChildNode->m_LeftChildNode;
            }

            return &leftMostChildNode->m_Pair;
        }

        return nullptr;
    }

    Pair<key_type, value_type>* end()
    {
        return nullptr;
    }

private:
    Pair<key_type, value_type>& GetNextPair(const Pair<key_type, value_type>& currentPair)
    {
        Node* currentNode = RecursivelyFind(m_RootNode, currentPair.m_Key);
        ASSERT_OR_DIE(currentNode != nullptr, "Node is null. Check allocation logic.");

        if (currentNode->m_RightChildNode != nullptr)
        {
            currentNode = currentNode->m_RightChildNode;

            while (currentNode->m_LeftChildNode != nullptr)
            {
                currentNode = currentNode->m_LeftChildNode;
            }
        }
        else
        {
            while (currentNode->m_ParentNode != nullptr && currentNode == currentNode->m_ParentNode->m_RightChildNode)
            {
                currentNode = currentNode->m_ParentNode;
            }

            currentNode = currentNode->m_ParentNode;
        }

        return currentNode->m_Pair;
    }

private:
    Node* m_RootNode;
};

And this is an example test use-case of this container:

int main()
{
    Map<int, float> myMap;
    myMap.Add(Pair<int, float>(0, 1.0f));
    myMap.Add(Pair<int, float>(33, 2.0f));
    myMap.Add(Pair<int, float>(7, 3.0f));
    myMap.Add(Pair<int, float>(47, 4.0f));
    myMap.Add(Pair<int, float>(99, 5.0f));
    myMap.Add(Pair<int, float>(61, 6.0f));

    for (const Pair<int, float>& currentPair : myMap)
    {
        DebuggerPrintf("Key: %i, Value: %f\n", currentPair.m_Key, currentPair.m_Value);
    }

    return EXIT_SUCCESS;
}

Now, this doesn't fail to compile, but it does crash at runtime (P.S.: The Add() is just a Map function I omitted here for minimal readability, it works fine).

I put a breakpoint in the pre-increment operator overload function. The breakpoint never actually hits. The custom overload does not get called. Instead, it seems like a pointer is getting incremented. More specifically, maybe a Pair pointer? And since the pointer is getting incremented like an integer, it ends up pointing at the wrong memory each iteration and eventually I get a read access violation because it's not pointing to the right memory:

Read Access Violation

I'm sure there's something wrong with my implementation. I'm not exactly sure where. What am I doing wrong here?

My goal here is not to find a means to an end, so I'm not looking for a quick solution like std::map or Boost libraries. I'm trying to learn how different data structures and containers are implemented using C++.


Solution

  • Your Pair is not an iterator.

    You're using Pair as a data-holding structure and as an iterator.

    In C++, iterators are distinct objects that represent positions within a container, and they need to support specific operations like incrementing, dereferencing, and comparing.

    You should create a separate Iterator class within your Map class to handle the traversal.

    By separating the Iterator functionality from the Pair class and implementing the necessary iterator operations, your container will work correctly with C++'s ranged-for loops. The iterator encapsulates the traversal logic and makes your Map container behave like a standard C++ container.