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

json::get function with argument #1227

Closed
ivankp opened this issue Sep 5, 2018 · 1 comment · Fixed by #1231
Closed

json::get function with argument #1227

ivankp opened this issue Sep 5, 2018 · 1 comment · Fixed by #1231
Assignees
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@ivankp
Copy link

ivankp commented Sep 5, 2018

The conversion operator template appears to not work for some types.
We just had a discussion about this on stack overflow (and an earlier one here).
Here's an example on Godbolt using GCC with -std=c++14. With -std=c++17 there's no error, but still a warning.

I would like to suggest a way for working around this.
Another version of the get function can be implemented like this:

template <typename T>
T& json::get(T& x) const { return x = get<T>(); }

so that the type may be deduced from the argument.

This would help particularly in situations where one has a reference to an object that needs to be given value from json, e.g.

auto& x = . . .;
json.get(x);

Which can be quite a bit less verbose than

auto& x = . . .;
x = json.get<std::decay_t<decltype(x)>>();

Thanks for the amazing library,
Ivan

@ivankp ivankp changed the title get function with type deduction json::get function with type deduction Sep 5, 2018
@ivankp ivankp changed the title json::get function with type deduction json::get function with argument Sep 5, 2018
@theodelrieu
Copy link
Contributor

Hello, I think it would be a good addition indeed, as it would reduce to_json/from_json implementations' verboseness.

The ideal use-case for the current get is creating the destination object directly, what you mention in your SO issue can be changed from type x; x = get<...>() to auto x = get<...>().

About the conversion operator, you can look at #958 for detailed information about why it doesn't always work.

theodelrieu added a commit to theodelrieu/json that referenced this issue Sep 10, 2018
Takes an lvalue reference, and returns the same reference.

This allows non-default constructible types to be used with get.
This overload does not require CopyConstructible either.

Implements nlohmann#1227
theodelrieu added a commit to theodelrieu/json that referenced this issue Sep 10, 2018
Takes an lvalue reference, and returns the same reference.

This allows non-default constructible types to be used with get.
This overload does not require CopyConstructible either.

Implements nlohmann#1227
theodelrieu added a commit to theodelrieu/json that referenced this issue Sep 12, 2018
Takes an lvalue reference, and returns the same reference.

This allows non-default constructible types to be used with get.
This overload does not require CopyConstructible either.

Conversion to C arrays is possible with this overload.

Implements nlohmann#1227
theodelrieu added a commit to theodelrieu/json that referenced this issue Sep 12, 2018
Takes an lvalue reference, and returns the same reference.

This allows non-default constructible types to be used with get.
This overload does not require CopyConstructible either.

Conversion to C arrays is possible with this overload.

Implements nlohmann#1227
theodelrieu added a commit to theodelrieu/json that referenced this issue Sep 28, 2018
Takes an lvalue reference, and returns the same reference.

This allows non-default constructible types to be converted without
specializing adl_serializer.
This overload does not require CopyConstructible either.

Implements nlohmann#1227
@nlohmann nlohmann self-assigned this Sep 29, 2018
@nlohmann nlohmann added this to the Release 3.2.1 milestone Sep 29, 2018
nlohmann added a commit that referenced this issue Sep 29, 2018
nlohmann added a commit that referenced this issue Sep 29, 2018
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Sep 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants