0

I have a map with the struct defined as under:

struct kv_string {
        std::string value;
        long long exp_time;
        kv_string(const std::string& v): value(v), exp_time(-1) {}
};

Now when I'm trying to insert a new structure using 

else if(qargs[0] == "set"){
    if(qargs.size()==3){
        kv_map.insert(std::make_pair( qargs[1], kv_string(qargs[2])));
    }
}

(qargs is a vector<string>), I get the following error:

> In file included from /usr/include/c++/4.8/map:61:0,
>                      from structures.h:5:
>     /usr/include/c++/4.8/bits/stl_map.h: In instantiation of ‘std::map<_Key, _Tp, _Compare, _Alloc>::mapped_type& std::map<_Key,
> _Tp, _Compare, _Alloc>::operator[](const key_type&) [with _Key = std::basic_string<char>; _Tp = kv_string; _Compare =
> std::less<std::basic_string<char> >; _Alloc =
> std::allocator<std::pair<const std::basic_string<char>, kv_string> >;
> std::map<_Key, _Tp, _Compare, _Alloc>::mapped_type = kv_string;
> std::map<_Key, _Tp, _Compare, _Alloc>::key_type =
> std::basic_string<char>]’:
>     /usr/include/c++/4.8/stdexcept:281:48:   required from here
>     /usr/include/c++/4.8/bits/stl_map.h:469:59: error: no matching function for call to ‘kv_string::kv_string()’
>                __i = insert(__i, value_type(__k, mapped_type()));
>                                                                ^
>     /usr/include/c++/4.8/bits/stl_map.h:469:59: note: candidates are:
>     structures.h:11:9: note: kv_string::kv_string(const string&)
>              kv_string(const std::string& v): value(v), exp_time(-1) {}
>              ^
>     structures.h:11:9: note:   candidate expects 1 argument, 0 provided
>     structures.h:8:8: note: kv_string::kv_string(const kv_string&)
>      struct kv_string {
>             ^
>     structures.h:8:8: note:   candidate expects 1 argument, 0 provided
>     make: *** [server_main.o] Error 1

I have also tried adding an additional constructor kv_string(){}, but it gives a segmentation fault.

Improve this question
0

2 Answers 2

Reset to default
3

You want this:

kv_map.insert(std::make_pair(qargs[1], kv_string(qargs[2]));

Or this:

kv_map.emplace(qargs[1], kv_string(qargs[2]);

Or, in C++17:

kv_map.try_emplace(qargs[1], qargs[2]);

The []-operator default-initializes a new element (if one doesn't exist for the given key), but your type kv_string is not default-constructible. So you cannot use that operator. The above operations are more powerful than the []-operator, too: they return an iterator to the element at the key, and information about whether the key already existed.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

Thanks! I didn't think that the problem could be here!
But using this is giving me a Segmentation fault.
@AyushGupta: So you probably have some bugs in some parts of code you haven't showed.
@Mr.C64 I didnt have any problems with the code anywhere else. When I enter this else if statement, I get a segmentation fault.
0

The C++ compiler emitted this error message complaining about the lack of a default constructor for your kv_string class:

/usr/include/c++/4.8/bits/stl_map.h:469:59: error: no matching function for call to ‘kv_string::kv_string()’
           __i = insert(__i, value_type(__k, mapped_type()));

If you read the documentation for std::map::operator[], you'll see that the mapped type (in your case kv_string) must be default-constructible.

So, if it makes sense for your own design, you could just add a default constructor to your kv_string struct:

struct kv_string {
    // ...

    // Default constructor
    kv_string() : exp_time(-1 /* or whatever default value */) {}
};

As a side note, I would also mark your kv_string(const std::string&) constructor as explicit, to avoid implicit conversions from strings.

Improve this answer

5 Comments

This seems like terrible advice. You don't generally want to add non-semantic noise to your interface to enable a meaningless and dangerous broken library expression somewhere. Just don't use that library API!
@KerrekSB: Your comment seems arrogant and as such stupid. Reserve the word "terrible" for your own stuff. If this is the style adopted on StackOverflow, that's a recipe for a failed community (actually, anti-community). Learn to be polite even when you disagree. If in the OP's design a default constructor makes sense, he can add it and continue using the []-operator, which is much more readable than other forms like std::map::insert(std::make_pair(...)), although I know the latter carries more information in the returned pair (assuming in the OP's code that information is relevant).
I'm mostly concerned with readers coming across this and picking up habits. It's nothing personal.
@KerrekSB: I honestly think []-operator allows writing very readable code, but I'm aware other methods are more versatile. I think it's the OP's choice whether making his class default-constructible to be able to use [] or not, basing on his own design. FWIW I upvoted your answer since I think it's well written and contains useful information.
Thanks. Well, operator[] does sometimes look nice, that's true, but unfortunately the map interface is a bit weird, and the operator not often appropriate. I'd really like to avoid suggesting to design your classes to work with certain library constructs. All too often people mangle semantics and violate requirements when they "just want to stick the type in a set", rather than designing the type to do what it was meant to do, and then worry about using it in containers later. A principled approach to design is very hard to teach...

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.