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

Add StringView template typename to basic_json #297

Closed
dv1 opened this issue Aug 19, 2016 · 3 comments
Closed

Add StringView template typename to basic_json #297

dv1 opened this issue Aug 19, 2016 · 3 comments
Labels
state: please discuss please discuss the issue or vote for your favorite option

Comments

@dv1
Copy link

dv1 commented Aug 19, 2016

Currently, basic_json expects one string type, which is used for everything - arguments, temporary strings, etc. This may be inefficient if data is available as some sort of a "string view". For example, if a C API offers a const char* value and a length integer, then it is possible to wrap these in a boost::string_ref instance. Since the JSON parser does not modify the source string, it would be possible to allow for specifying something like boost::string_ref as a "StringView" template typename, and std::string as a "String" type, which is then used internally.

The net gain is that a copy is avoided. In the example mentioned above, I have to create a temporary std::string and do a copy of the source string. With a StringView typename, I could avoid that copy.

@nlohmann
Copy link
Owner

I don't fully understand. Where exactly would this be added?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Aug 24, 2016
@dv1
Copy link
Author

dv1 commented Sep 2, 2016

I'd add a json::parse() overload that accepts string views.

EDIT: Related: a templated parse() variant that passes through universal references would also be useful. See for example this article about perfect forwarding and universal references.

@nlohmann
Copy link
Owner

nlohmann commented Nov 8, 2016

I need more information on this to assess whether and how to extend the library.

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

No branches or pull requests

2 participants