Discuss: replace static "iterator_wrapper" function with "items" member function

[nlohmann]

The static iterator_wrapper function is a means to access objects' keys in a range for. Example:

json j_object = {{"one", 1}, {"two", 2}};

for (auto& x : json::iterator_wrapper(j_object)) {
    std::cout << "key: " << x.key() << ", value: " << x.value() << '\n';
}

I never was happy about the name of the function and there were several issues where users expected json to behave as a map so that .first and .second could be used.

Question 1

As for the name: What about replacing the static iterator_wrapper function by a member function items (name motivated by Python)?

Usage:

json j_object = {{"one", 1}, {"two", 2}};

for (auto& x : j_object.items()) {
    std::cout << "key: " << x.key() << ", value: " << x.value() << '\n';
}

Question 2

One could even think about supporting .first and .second, but this would mean pre-computing the values for .key() and .value() which would mean a bit of overhead.

What do you think?

[gregmarr]

I think adding the .items() would be useful to make it less verbose. Having .first and .second would mean that you'd have to add a std::string and a std::reference_wrapper to the type. This would increase the size, and potentially change semantics of .value(), because the type would be something like std::reference_wrapper<std::remove_reference<IteratorType::reference>::type> instead of just IteratorType::reference. The semantics are already a bit different than iterating over a map as it allows you to iterate over an array and get the indices as strings, so I don't think having .key() and .value() makes it that much more difficult to use.

[nlohmann]

Any further comments?

[gregmarr]

Oh, thanks for referencing this, it points out that I missed something in my evaluation on the deduction guides issue.

[gregmarr]

Just one more: we don't necessarily need to remove .value() or make it return .second, so we can do this without changing the semantics of .value().

I'd say add .items() now. It's independent from .first/.second and we can continue that bit via #901.

[nlohmann]

Instead of adding .items(), I would like to replace iterator_wrapper with it. I think iterator_wrapper should be deprecated with the next release.

[gregmarr]

To follow semver, we need to deprecate it now and remove it in 4.0.

[nlohmann]

Right, this is what I meant.

[nlohmann]

iterator_wrapper is now deprecated and will be removed in 4.0.0.

[Anthony-J-Garot]

A note in https://github.com/nlohmann/json/blob/develop/doc/examples/iterator_wrapper.cpp would be helpful.

[nlohmann]

@Anthony-J-Garot I did so in be1eafa which should be merged to develop soon(ish).