Question

I am playing with container types, gettingmore into the details and try to build an unmodifiable (or immutable) map.

For that, I built some kind like copy-constructor to add all elements from another unordered_map into my UnmodifiableMap (which is basically a wrapper for std::unordered_map). To make it unmodifiable, I only provide const iterators and read-only methods. But I get stuck with the constructor, I am sure I miss something, maybe somebody can point me the problem here. Maybe it is a totally wrong way like this, but his is what tried so far:

template <typename Key, typename T,
    typename HashFcn = std::hash<Key>,
    typename EqualKey = std::equal_to<Key>,
    typename Alloc = std::allocator<std::pair<const Key, T> > > 
class UnmodifiableMap {
public:
    // The actual data
    typedef std::unordered_map<T, Key, HashFcn, EqualKey> base;

    typedef Key key_type;
    typedef T data_type;
    typedef T mapped_type;
    typedef std::pair<const key_type, data_type> value_type;
    typedef HashFcn hasher;
    typedef EqualKey key_equal;
    typedef Alloc allocator_type;

    typedef typename base::size_type size_type;
    typedef typename base::const_iterator const_iterator;
    typedef typename base::iterator iterator;

private:
    base _map;

    /**
     * Constructs an empty unordered_map 
     */
    UnmodifiableMap(
        size_type n = 0, const hasher& hf = hasher(),
        const key_equal& eql = key_equal(),
        const allocator_type &alloc = allocator_type())
            : _map(n, hf, eql, alloc) {}

public:
    /** Constructs a copy of unordered_map */
    UnmodifiableMap(const base& other)
    : _map(static_cast<const base&>(other)) {}

    ~UnmodifiableMap() {}   

    iterator begin() { return _map.begin(); }
    iterator end() { return _map.end(); }
    const_iterator begin() const { return _map.begin(); }
    const_iterator end() const { return _map.end(); }

    bool empty() const { return _map.empty(); }

    bool contains(const key_type& key) const { 
        return _map.find(key) != _map.end(); }
};

And here the main body:

int main(int argc, char **argv) {
    typedef std::unordered_map<int, std::string> Items;
    Items map;

    map[1] = "first string";
    map[4] = "string 4";
    map[5] = "string 5";
    map[22] = "string 22";
    map[12] = "string 12";
    map[18] = "string 18";

    typedef UnmodifiableMap<int, std::string> ReadOnlyItems;
    ReadOnlyItems readonlymap(map);

    return 0;
}

The error I get is

Unmodifiable_map.cpp: In function ‘int main(int, char**)’:
Unmodifiable_map.cpp:56:25: error: no matching function for call to ‘UnmodifiableMap<int, std::basic_string<char> >::UnmodifiableMap(Items&)’
Unmodifiable_map.cpp:56:25: note: candidates are:
Unmodifiable_map.h:45:2: note: UnmodifiableMap<Key, T, HashFcn, EqualKey, Alloc>::UnmodifiableMap(const base&) [with Key = int, T = std::basic_string<char>, HashFcn = std::hash<int>, EqualKey = std::equal_to<int>, Alloc = std::allocator<std::pair<const int, std::basic_string<char> > >, UnmodifiableMap<Key, T, HashFcn, EqualKey, Alloc>::base = std::unordered_map<std::basic_string<char>, int, std::hash<int>, std::equal_to<int>, std::allocator<std::pair<const std::basic_string<char>, int> > >]
Unmodifiable_map.h:45:2: note:   no known conversion for argument 1 from ‘Items {aka std::unordered_map<int, std::basic_string<char> >}’ to ‘const base& {aka const std::unordered_map<std::basic_string<char>, int, std::hash<int>, std::equal_to<int>, std::allocator<std::pair<const std::basic_string<char>, int> > >&}’
Unmodifiable_map.h:37:2: note: UnmodifiableMap<Key, T, HashFcn, EqualKey, Alloc>::UnmodifiableMap(UnmodifiableMap<Key, T, HashFcn, EqualKey, Alloc>::size_type, const hasher&, const key_equal&, const allocator_type&) [with Key = int, T = std::basic_string<char>, HashFcn = std::hash<int>, EqualKey = std::equal_to<int>, Alloc = std::allocator<std::pair<const int, std::basic_string<char> > >, UnmodifiableMap<Key, T, HashFcn, EqualKey, Alloc>::size_type = long unsigned int, UnmodifiableMap<Key, T, HashFcn, EqualKey, Alloc>::hasher = std::hash<int>, UnmodifiableMap<Key, T, HashFcn, EqualKey, Alloc>::key_equal = std::equal_to<int>, UnmodifiableMap<Key, T, HashFcn, EqualKey, Alloc>::allocator_type = std::allocator<std::pair<const int, std::basic_string<char> > >]
Unmodifiable_map.h:37:2: note:   no known conversion for argument 1 from ‘Items {aka std::unordered_map<int, std::basic_string<char> >}’ to ‘long unsigned int’
Unmodifiable_map.h:14:7: note: UnmodifiableMap<int, std::basic_string<char> >::UnmodifiableMap(const UnmodifiableMap<int, std::basic_string<char> >&)
Unmodifiable_map.h:14:7: note:   no known conversion for argument 1 from ‘Items {aka std::unordered_map<int, std::basic_string<char> >}’ to ‘const UnmodifiableMap<int, std::basic_string<char> >&’
Unmodifiable_map.h:14:7: note: UnmodifiableMap<int, std::basic_string<char> >::UnmodifiableMap(UnmodifiableMap<int, std::basic_string<char> >&&)
Unmodifiable_map.h:14:7: note:   no known conversion for argument 1 from ‘Items {aka std::unordered_map<int, std::basic_string<char> >}’ to ‘UnmodifiableMap<int, std::basic_string<char> >&&’

Hope somebody can shed some light on that. Also I think I need to do more in the copy constructor, propably copy the elements (like a swap function) and an implementaton of operator= ?!

Was it helpful?

Solution

You are swapping the role of T and Key in your base type definition inside your wrapper class:

template <typename Key, typename T,
    typename HashFcn = std::hash<Key>,
    typename EqualKey = std::equal_to<Key>,
    typename Alloc = std::allocator<std::pair<const Key, T> > > 
class UnmodifiableMap {
public:
    // typedef std::unordered_map<T, Key, HashFcn, EqualKey> base; // ERROR
    typedef std::unordered_map<Key, T, HashFcn, EqualKey> base; // OK
    ...

Hence, your underlying map ends up being an unordered_map<string, int> rather than an unordered_map<int, string>, and the compiler complains about the mismatch.

Also notice, that you have an unnecessary static_cast in your constructor:

UnmodifiableMap(const base& other)
// : _map(static_cast<const base&>(other)) {} // NOT NEEDED. Just do:
   : _map(other) {}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top