How can I return `map::iterator`? (`map` class extends `std::map`)

Question

I making a custom map class (kc::map) that extends std::map, and I want to add at_index(), and I need to return map<K, V>::iterator. But when I return map<K, V>, this error occurs: error: invalid use of incomplete type ‘class kc::map<K, V>’:

namespace kc
{
    template<typename K, typename V> class map : public std::map<K, V>
    {
        public:
            using std::map<K, V>::map;
            map<K, V> get_value()
            {
                return *this;
            }
            map<K, V>::iterator at_index(int index)
            {
                map<K, V> m = get_value();
                for (int i = 0; i < index; i++)
                {
                    m.erase(m.begin());
                }
                return m.begin();
            }
    };
};

Do you want it to return `kc::map::iterator` (which doesn't exist in your shown code) or do you want it to return `std::map::iterator`? And you should probably ask yourself the same for the other mentions of `map` in your code as well. — user17732522, Jan 18 '22 at 08:04
Deriving from standard classes is rarely a good idea. Even if you get this to compile you're returning an iterator to a local map which will cease to exist at the end of the function — Alan Birtles, Jan 18 '22 at 08:06
https://quuxplusone.github.io/blog/2018/12/11/dont-inherit-from-std-types — selbie, Jan 18 '22 at 08:06
https://stackoverflow.com/questions/2034916/is-it-okay-to-inherit-implementation-from-stl-containers-rather-than-delegate — selbie, Jan 18 '22 at 08:07
Your `at_index` function returns an iterator to a **copy** of the map that has already been destructed by the time the function returns the iterator to the caller. — selbie, Jan 18 '22 at 08:08
the standard library offers different customization points for working with containers. Inheriting from them isnt one of them. I dont quite understand what the code does, but it seems it could be an alogrithm taking two iterators and returning one — 463035818_is_not_an_ai, Jan 18 '22 at 08:12
See you really shouldn't inherit from STL (as I already mentioned in your previous question : https://stackoverflow.com/questions/70751096/how-can-i-multiply-a-map-with-operator-overloading/70751202) — Pepijn Kramer, Jan 18 '22 at 08:12

score 2 · Answer 1 · answered Jan 18 '22 at 08:27

You could return a std::map<K,V>::iterator. This would make your code compile, but the iterator would be invalid. get_value returns a copy, m is local to at_index and gets destroyed when the function returns.

Inheriting publicly from standard containers is rarely a good idea. Standard containers are not made to be inherited from (eg they do not have a virtual destructor). You can write at_index as an algorithm:

#include <map>
#include <iostream>

template <typename IT>
IT get_at_index(IT begin,IT end,unsigned index){
    if (std::distance(begin,end) < index) return end;
    return std::next(begin,index);
}


int main() {
    std::map<int,int> m{{1,1},{2,2},{3,3}};
    auto it = get_at_index(m.begin(),m.end(),1);
    if (it != m.end()) std::cout << it->first << " " << it->second << "\n";

    it = get_at_index(m.begin(),m.end(),5);
    if (it == m.end()) std::cout << "index 5 is out of range\n";
}

However, a std::map isnt meant to be accessed by index. It is rather expensive to advance a maps iterator. If you want a container of key-value pairs that you can access by index, consider to use a std::vector<std::pair<K,V>> instead.

note that `std::map::iterator` needs a `typename` as it's a template dependant type — Alan Birtles, Jan 18 '22 at 08:33

Alan Birtles · Accepted Answer · 2022-01-18T10:32:43.857

Ignoring the real problems with the code (deriving from standard containers is not recommended and at_index returns a dangling iterator to the local object m) to get the code to compile you have two options.

within a class you don't need to prefix members of the class with the class name. As iterator isn't a class member you need to add it first then an unqualified iterator will work:

using iterator = typename std::map<K, V>::iterator;
iterator at_index(int index)

You can just use std::map::iterator directly:

typename std::map<K, V>::iterator at_index(int index)

If what you're actually trying to do is get the ith item in a std::map this will work:

#include <map>
#include <stdexcept>
#include <iostream>

namespace kc
{
    template<typename K, typename V> class map
    {
        private:
            std::map<K, V> impl;

        public:
            using iterator = typename std::map<K, V>::iterator;
            using value_type = typename std::map<K, V>::value_type;

            std::pair<iterator, bool> insert( const value_type& value )
            {
                return impl.insert(value);
            }
           
            iterator at_index(int index)
            {
                if (index >= impl.size())
                {
                    throw std::invalid_argument("index out of range");
                }
                auto it = impl.begin();
                std::advance(it, index);
                return it;
            }
    };
};

int main()
{
    kc::map<int, int> m;
    m.insert({1, 1});
    m.insert({2, 2});
    m.insert({3, 3});
    std::cout << m.at_index(2)->second;
}

Isn't that possible to return `kc::map` instead of `std::map`? — NoIdeaForUsername, Jan 18 '22 at 09:32
Thanks, just used `typename map::iterator` instead of `map::iterator`, and it works! Thanks — NoIdeaForUsername, Jan 18 '22 at 10:42

How can I return `map::iterator`? (`map` class extends `std::map`)

2 Answers2