0

I have written a class to wrap around an std::map, along with an iterator class to allow controlling accessing the map's key:

#include <map>

class Container
{
public:
    class iterator
    {
        using map = std::map < int, int >;
        map::const_iterator _iter;

    public:
        iterator() {}
        iterator(map::iterator iter) :_iter(iter) {}
        iterator(map::const_iterator iter) :_iter(iter) {}
        iterator(const iterator& b) :_iter(b._iter) {}

        iterator& operator=(const iterator& b)
        {
            _iter = b._iter;
            return *this;
        }

        iterator& operator++()
        {
            ++_iter;
            return *this;
        }

        iterator operator++(int)
        {
            return iterator(_iter++);
        }

        const map::key_type first()
        {
            return std::abs(_iter->first);
        }

        const int second()
        {
            return _iter->second;
        }

        bool operator==(const iterator& b)
        {
            return _iter == b._iter;
        }

        bool operator!=(const iterator& b)
        {
            return _iter != b._iter;
        }
    };

    void insert(int key, int value)
    {
        _map[-key] = value;  // Key is modified from user, hence need for wrapper
    }

    iterator begin()
    {
        return iterator(_map.begin());
    }

    iterator end()
    {
        return iterator(_map.end());
    }

    iterator find(int key)
    {
        return iterator(_map.find(key));
    }

    iterator erase(iterator iter)
    {
        return iterator(_map.erase(iter));
    }

private:

    std::map<int, int> _map;
};

My required operations are:

  • Iterating
  • Finding
  • Erasing
  • Getting key
  • Getting value

I would like usage to be like:

int main()
{
    Container o;
    o.insert(-1, 100);
    o.insert(-2, 200);
    o.insert(-3, 300);
    o.insert(-4, 300);

    for (Container::iterator i = o.begin(); i != o.end();)
    {
        if (i.first() == -2)
        {
            std::cout << i.first() << " " << i.second() << std::endl;
            ++i;
        }
        else
        {
            i = o.erase(i);
        }
    }
}

so that I can erase elements and continue iterating.

However, I am struggling to implement the container's erase() method because the input is my custom iterator type, not the map's key_type.

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
user997112
  • 29,025
  • 43
  • 182
  • 361
  • 2
    Why do all this wrapping? Why not just write a free function so you can say `insert(o, -1, 100)` and use the map class as-is? – John Zwinck Aug 19 '18 at 02:30
  • @JohnZwinck Because I am modifying how the key is stored and the user is not aware of this, so I need to prevent them from having direct access. I will return the unmodified key when they call iterator.first(). – user997112 Aug 19 '18 at 02:32
  • Your `Container::iterator` contains a `map::iterator`, pass that to `map::erase`. Why is `Container::iterator` default constructible? As the other comment says, this wrapper seems unnecessary. – Praetorian Aug 19 '18 at 02:32
  • 1
    Can you explain in more detail what the motivation is? Your example does not do what you say--it stores the key unchanged. – John Zwinck Aug 19 '18 at 02:33
  • @Praetorian I used example custom iterator code I found on SO. Have explained in previous comment the need for this class. – user997112 Aug 19 '18 at 02:33
  • @JohnZwinck I originally wanted to have two std::maps, one I iterate backwards, the other forwards, using the same code. However, map.erase() doesn't work nicely with reverse_iterators, so I had the idea to store the keys in their negated form, which would allow me to iterate forwards, with the effect of iterating backwards. I need to ensure the user never retrieves the negated key, hence I need to wrap the map. – user997112 Aug 19 '18 at 02:35
  • Have you seen this question about how to erase a reverse_iterator? https://stackoverflow.com/questions/1830158/how-to-call-erase-with-a-reverse-iterator - again you can just write a simple free function like `erase(map, reverse_iterator)` to solve your real problem. – John Zwinck Aug 19 '18 at 02:38
  • @user997112 No need to negate the key, you can use the opposite ordering relation (which is not the negation). Bonus, that works on keys that don't have an opposite. – curiousguy Aug 19 '18 at 02:41
  • @curiousguy assume you mean constructing using std::greater? – user997112 Aug 19 '18 at 04:50
  • @user997112 Yes `greater` or another inverted relation. The point is that it's very easy to forget to negate a number at some point in the code, but when you have chosen an ordering relation for a container, you don't need to do anything more. – curiousguy Aug 19 '18 at 05:20

1 Answers1

3

Declare class Container as friend class of Iterator, and access the _iter in erase function.

class iterator
{
    friend class Container;
    //...
}

iterator Container::erase(iterator iter)
{
    return iterator(_map.erase(iter._iter));
}
rsy56640
  • 299
  • 1
  • 3
  • 13