-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Adding first and second properties to iteration_proxy_internal #578
Adding first and second properties to iteration_proxy_internal #578
Conversation
Here's the test that I used: #include <json.hpp>
#include <iostream>
using namespace std;
using namespace nlohmann;
int main()
{
json j = {{"A", 1}, {"B", 2}};
for (auto i : json::iterator_wrapper(j))
{
auto x = i.first; // x = "A", x = "B"
auto y = i.second; // y = 1, y = 2
cout << x << ", " << y << endl;
}
} Here's the output:
|
This is a nice solution! Thanks! |
Unfortunately, the code does not work for me. If you look into the test suite in It would be great if you could have a look at this. |
I'm using MSVC 2017. What compiler failed for you? |
clang version 5.0.0 (trunk 292523) (llvm/trunk 292718) - in any case, it would be great to have unit tests - I should not have merged the code without. |
I'll look into it, and I'll see about adding the unit tests as well. |
That would be great! |
This PR fixes #350 by adding property-like members to the iterator_proxy_internal class.
The fix proposed in that issue looks as if you pay for it even if you don't use .first or .second (at least the .first shows a copy of a string each time the iterator is incremented). That cost shouldn't have to be payed, but we don't have to sacrifice the interface that we want.
This solution provides the .first and .second interface as outlined in the issue, but shouldn't cost anything, and is likely inlined out of existence after compilation.