Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request: 'emplace_back' #349

Closed
jaredgrubb opened this issue Nov 2, 2016 · 10 comments
Closed

Request: 'emplace_back' #349

jaredgrubb opened this issue Nov 2, 2016 · 10 comments

Comments

@jaredgrubb
Copy link
Contributor

Out of habit, I tend to use 'emplace_back' instead of 'push_back' and I was surprised basic_json didn't support this. Can we add it?

@nlohmann
Copy link
Owner

nlohmann commented Nov 2, 2016

I have to check whether this makes sense, but you're right that not having it, but having push_back seems odd.

@jaredgrubb
Copy link
Contributor Author

I think it's considered part of the various "container" concepts (http://en.cppreference.com/w/cpp/concept/SequenceContainer). Having it missing isn't that big a deal, but would be nice.

@nlohmann nlohmann self-assigned this Nov 11, 2016
@nlohmann nlohmann added this to the Release 2.0.8 milestone Nov 11, 2016
@nlohmann
Copy link
Owner

Commit 1be73b9 contains a proposal for an emplace_back function (for arrays) and an emplace function (for objects). These functions more or less mimic the behavior of push_back (which is overloaded to add to an array if a basic_json value is passed and to an object if an basic_json::object_t is passed).

It would be nice to discuss this proposal:

  • Is the implementation as expected?
  • Is it OK to have two functions?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Nov 11, 2016
@jaredgrubb
Copy link
Contributor Author

The implementation looks correct. Although on the one hand I think it's odd for the json object to have both methods, it matches the convention that json has object and array methods and it's up to you to call the right ones. So it looks good to me!

@nlohmann
Copy link
Owner

Thanks. Yes, it is a weird situation to have a combination of a map and a vector, and I am not sure whether users have the mindset "I am using a general container" or rather "I am using an object" / "I am using an array".

Another aspect: If there was a single emplace function, it could forward the arguments to to the underlying vector or map depending whether the type of the JSON value is and array or object. This would make the automatic translation of null into array or object as it is done in push_back impossible.

@nlohmann
Copy link
Owner

Any opinions on this?

@jaredgrubb
Copy link
Contributor Author

There are times when I wish I had a stronger type that represented what I know it is but I can see the utility of the do-everything type. I dont see a strong reason to change it myself! So The emplace_back would be awesome :)

@TurpentineDistillery
Copy link

Note that std::map::emplace has a return type that is not void. Was it a conscious decision not to do the same in basic_json-object ?

@nlohmann
Copy link
Owner

@TurpentineDistillery Good point! I implemented emplace_back before (which is void until C++17) and totally forgot about the return value. I shall fix this.

@nlohmann
Copy link
Owner

nlohmann commented Dec 1, 2016

Merged.

@nlohmann nlohmann closed this as completed Dec 1, 2016
@nlohmann nlohmann removed the state: please discuss please discuss the issue or vote for your favorite option label Dec 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants