Wrapper for a standard STL-container

Question

In the codebase I support, I found the following code. I am interested in your opinion about its expediency and especially the expediency of redefining the operator().

  template <typename KEY, typename VALUE>
  class MyUnorderedMap {
    std::unordered_map<KEY, VALUE> map;

  public:
    template <typename T>
    auto insert(const T value) {
      return map.insert(value);
    }
    inline auto& operator()(const KEY& r) { return (*this)[r]; }
    inline auto& operator[](const KEY r) { return map[r]; }
    inline auto find(const KEY& key) const { return map.find(key); }
    inline auto size() const { return map.size(); }
    inline auto begin() { return map.begin(); }
    inline auto end() { return map.end(); }

    inline bool exists(const KEY& userId) const {
      return map.find(userId) != map.end();
    }
    bool remove(const KEY& userId) {
      auto it = map.find(userId);
      bool retval = (it != map.end());
      if (retval)
        map.erase(it);
      return retval;
    }
  };

Answer 1

I question the value of this template class. We hide many important members of std::unordered_map (including crbegin(), empty(), reserve(), at(), try_emplace()) for no good reason. The additional functions that are provided have no business being members: they are perfectly implementable as free functions without privileged access.

Considering the members in turn:

• insert(): only instantiable if T is convertible to the map's value_type. Unnecessarily copies the argument.
• operator(): unnecessary, given that we have operator[].
• operator[]: unnecessarily copies its argument.
• find(), size() begin(), end(): required only because the map is private.
• exists(): should be a free function. If this is generic, why is the parameter called userId?
• remove() - why not just use the erase() that accepts a key? Or do we have to build with pre-C++11 library?

---

When I've wanted to provide something like the exists() here, I've made it a standalone template function that can work with many kinds of container:

/*
 * contains()
 * A convenience overload that searches from beginning to end of a container.
 *
 * If an iterator is supplied, it will be written with the found position or
 * container.end() as appropriate.
 */
template<typename T, typename U>
inline bool contains(const T& container, const U& value)
{
    using std::begin;
    using std::end;
    return find(begin(container), end(container), value) != end;
}

template<typename T, typename U, typename Iter>
    /* Iter should be a T::iterator or T::const_iterator */
inline bool contains(T& container, const U& value, Iter& it)
{
    using std::begin;
    using std::end;
    return (it = find(begin(container), end(container), value)) != end;
}

/*
 * Overloads for pointer-to-const values within containers of pointer-to-mutable.
 */
template<typename T, typename U>
inline bool contains(const T& container, const U* value)
{
    using std::begin;
    using std::end;
    return find(begin(container), end(container), value) != end;
}

template<typename T, typename U, typename Iter>
inline bool contains(const T& container, const U* value, Iter& it)
{
    using std::begin;
    using std::end;
    return (it = find(begin(container), end(container), value)) != end;
}

/*
 * Overloads for maps (by key)
 */
template<typename T>
inline bool contains(const T& container, const typename T::key_type& key)
{
    return container.find(key) != container.end();
}

template<typename T, typename Iter>
inline bool contains(T& container, const typename T::key_type& key, Iter& it)
{
    return (it = container.find(key)) != container.end();
}

template<typename T>
inline bool contains(const T& container, const typename T::key_type& key,
                     typename T::mapped_type& result)
{
    typename T::const_iterator it;
    return contains(container, key, it) && (result = it->second, true);
}