Storing passed objects in map by unique_pointer

Question

I have the following problem in a c++-project at my university:
I want to store some objects of Class1 by using a map. Because of the design of my project the class that holds the map as a member variable does not create the objects to store in the map. My research on this forum tells me to use unique_ptr to pass those objects.

When combining those two ideas in a test projects for experiments:

#include <iostream>
#include <memory>
#include <unordered_map>

typedef std::unique_ptr<std::string> string_ptr;

class Class1 {
public:
    std::unordered_map<std::string, string_ptr> my_unordered_map;

    Class1() : my_unordered_map(){
    }

    void addTo(string_ptr ptr) {
        std::string string = *ptr;
        std::cout << string << std::endl;
        my_unordered_map[0] = std::move(ptr);
    }
};


int main() {
    std::cout << "Process started!"<< std::endl;

    string_ptr ptr;
    ptr = std::make_unique<std::string>("text");

    Class1 cont;
    cont.addTo(std::move(ptr));


}

it compiles, but throws a

terminate called after throwing an instance of 'std::logic_error'
what(): basic_string::_M_construct null not valid

at runntime.

I also tried changing void addTo(string_ptr ptr) to addTo(string_ptr &ptr) as suggested in other posts, but the problem remained the same.

Now, whats the problem? Is it:
a) A design mistake to not let Class1 create the objects by itself. C++ inserting unique_ptr in map would answer my question in this case.
-- or --
b) Something else my small knowledge of c++ lets me miss?

Sidenote: ofc the objects i want to store in those pointers are not strings.

As mentioned at the end the objects i want to store are not strings. Or am I missing your point? — CharJ, Aug 22 '17 at 18:46
The point is there is most likely no reason to store `unique_ptr` instead of `string`. — juanchopanza, Aug 22 '17 at 18:55
@Benjamin Well, I thought not regarding OP's real situation. But you're right, we encourage to eliminate the irrelevant stuff. — user0042, Aug 22 '17 at 18:57
@juanchopanza As far as my knowledge goes: Storing strings will call the copy constructor to do so. This would require me to rewrite it to avoid long running times for big objects, wont it? — CharJ, Aug 22 '17 at 20:34
If you can avoid copy constructing unique_ptrs, you can avoid copy constructing strings. — juanchopanza, Aug 22 '17 at 20:51
As I'm trying to improve my C++ knowledge I again made some testing. I think I'm not quite getting your point, this is what I did: http://codepad.org/Fe1bXUPO — CharJ, Aug 22 '17 at 22:13

score 2 · Answer 1 · answered Aug 22 '17 at 18:50

2

Your problem has nothing to do with unique_ptr. It has nothing to do with your value object at all. It is actually with your key. Unfortunately, the integer literal 0 (and only 0, and not an int variable with the value 0) can be treated as a pointer of any type. And also unfortunately, std::string has an implicit conversion constructor from char const*. So when you do this:

my_unordered_map[0] = std::move(ptr);

Since your key type is std::string, you are implicitly constructing a std::string from an null pointer. That is your error.

answered Aug 22 '17 at 18:50

Benjamin Lindley

101,917
9
204
274

Why did you post same answer? – kocica Aug 22 '17 at 18:59
1

@FilipKočica: I didn't. I was writing my answer before yours was posted. And when yours finally was initially posted, it did not explain the situation satisfactorily. You simply said "you can't do this". In fact you can do it. And he did. – Benjamin Lindley Aug 22 '17 at 19:01
In my opinion it does explain it enough and solved his problem even before you posted it :-) – kocica Aug 22 '17 at 19:08
@FilipKočica: You're welcome to that opinion. I don't share it. – Benjamin Lindley Aug 22 '17 at 19:10
OK have a nice day. – kocica Aug 22 '17 at 19:16
While @FilipKočicas answer solved my problem this is indeed some interesting insight about why it even compiles. Thank you! – CharJ Aug 22 '17 at 19:54

kocica · Accepted Answer · 2017-08-22T19:15:20.950

1

Here

my_unordered_map[0] = std::move(ptr);

you are passing int as key, but my_unordered_map expects std::string as key, solution:

my_unordered_map[string] = std::move(ptr);

terminate called after throwing an instance of 'std::logic_error' what(): basic_string::_M_construct null not valid

std::string has implicit conversion constructor from char const*, it means attempt to construct std::string from NULL pointer, what causes this error.

If you would need empty string, do following

my_unordered_map[std::string("")] = std::move(ptr);

edited Aug 22 '17 at 19:15

answered Aug 22 '17 at 18:47

kocica

6,412
2
14
35

I doubt using an empty string as key is what the OP actually wants. – user0042 Aug 22 '17 at 18:48
Oh my god. Not even coding for 10hrs straight is an excuse for this fail. As i never worked with uniqu_ptrs before i was expecting some complicated reason for this cryptic error massage. In fact it was just me using 0 instead of a valid string... Thank you for your time! – CharJ Aug 22 '17 at 18:49
It happens :-) everything ok now ? – kocica Aug 22 '17 at 18:52

Storing passed objects in map by unique_pointer

2 Answers2