-7

I'm basically trying to create a thread-safe wrapper class for std::map. As I'm coming from C, I have quite a hard time figuring out all the nuances of C++. I'm trying to overwrite the [] operator to take std::string arguments to just pass it to my std::map member. By the reference of std::map::operator[] this should work fine: 

T& operator[]( const Key& key );

Here is my class:

thread_map.hpp

#ifndef THREAD_MAP_H
#define THREAD_MAP_H

#include <map>
#include <functional>
#include <mutex>

template <class T>class Thread_map
{
private:
    std::map<std::string, T> map;
    std::mutex map_mutex;

public:

    ~Thread_map();

    T& at(size_t pos);
    T& operator[](std::string &key);

    size_t size() const;
    bool empty() const;

    void clear();
    void insert(std::pair<std::string, T> pair);
    T& erase(const std::string &key);

    bool for_each(std::function<bool (Thread_map, std::string&, T&)> fun);
};

template<class T> Thread_map<T>::~Thread_map()
{
    this->map.clear();
}

template<class T> T& Thread_map<T>::at(size_t pos)
{
    T *value;
    this->map_mutex.lock();
    value = this->map.at(pos);
    this->map_mutex.unlock();
    return value;
}

template<class T> T& Thread_map<T>::operator[](std::string &key)
{
    this->map_mutex.lock();
    T &value = this->map[key];
    this->map_mutex.unlock();
    return value;
}

template<class T> size_t Thread_map<T>::size() const
{
    size_t size;
    this->map_mutex.lock();
    size = this->map.size();
    this->map_mutex.unlock();
    return size;
}

template<class T> bool Thread_map<T>::empty() const
{
    bool empty; 
    this->map_mutex.lock();
    empty = this->map.empty();
    this->map_mutex.unlock();
    return empty;
}

template<class T> void Thread_map<T>::clear()
{
    this->map_mutex.lock();
    this->map.clear();
    this->map_mutex.unlock();
}

template<class T> void Thread_map<T>::insert(std::pair<std::string, T> pair)
{
    this->map_mutex.lock();
    this->map.insert(pair);
    this->map_mutex.unlock();
}

template<class T> T& Thread_map<T>::erase(const std::string &key)
{
    T *value;
    this->map_mutex.lock();
    value = this->map.erase(key);
    this->map_mutex.unlock();
    return value;
}

template<class T> bool Thread_map<T>::for_each(std::function<bool 
(Thread_map, std::string&, T&)> fun)
{

}


#endif

I put the implementation into the header file, because I heard you do that with template classes. Am I right on that? My Problem is, that when I try to call the operator

Thread_map<std::string> map;
map["mkey"] = "value";

g++ throws an invalid initialization error on map["mkey"]. As far as my understanding goes the problem is that mkey gets compiled to std::string("mkey") which is just the value, not a reference. But why or how does the following work then?

std::map<std::string, std::string> map;
map["mkey"] = "value";

I mean I could just pass the string by value but that seems inefficient.

Student
  • 805
  • 1
  • 8
  • 11
pear
  • 1
  • 3
  • 1
    Compare and contrast `T& operator[]( const Key& key )` with `T& Thread_map::operator[](std::string &key)`. There's one keyword that's present in the former and missing in the latter. Hint: it begins with `c` and ends with `t`. – Igor Tandetnik Jul 22 '18 at 17:26
  • Well thanks overlooked that :D – pear Jul 22 '18 at 17:31
  • 1
    Your code seems to be riddled with errors. – Killzone Kid Jul 22 '18 at 18:54
  • Can you tell me what the errors are or not Kkid. Im kinda new to C++? Well ill refere you to that link Fei Xiang: https://stackoverflow.com/questions/8752837/undefined-reference-to-template-class-constructor – pear Jul 22 '18 at 19:04
  • 1
    @pear Try to `erase` a key from your dictionary, use `empty` function, use `size` function (An hint- `empty` and `size` problems are the same kind), and use `at` function. – Coral Kashri Jul 22 '18 at 19:15
  • @pear Ignore what I previously said. I made a mistake, my bad. – eesiraed Jul 27 '18 at 04:06

1 Answers1

-2

Reference requires a variable's address that cans be modified. For lvalue string ("some string") there is no address that can be value modified. There are some ways that I know to solve this issue:

Remove Reference (Not recommended)

One way is to remove the '&' from the parameter. like this:

T& operator[](std::string key);

In this way you don't request lvalue, but rvalue. The problem is that when ever you'll send a value, you won't send 4 bytes of memory address, but sizeof("Your string") bytes. Heavy method..

Make a const lvalue (Recommended way)

The most beautiful way to solve this issue, is to make the parameter const lvalue (what that called rvalue reference), to make a promise to the compiler that you won't try to change the given address's value inside this function. It's looks like this:

T& operator[](const std::string &key);

Now you can send a lvalue string and rvalue string.

Give up

This way is not bad as the first one, but absolutely not good as the second. You can easily use your declaration:

T& operator[](std::string &key);

And when you pass a value, use another string variable to store the value, and use this variable at call time:

Thread_map<std::string> map;
string key = "mkey";
map[key] = "value";

(Don't do that.. Just as extension for the knowledge).

Coral Kashri
  • 3,436
  • 2
  • 10
  • 22