Request: range-based-for over a json-object to expose .first/.second

[jaredgrubb]

I would love to have the ability to iterate over a JSON object type in exactly the same way I would iterate over a std::map:

for(auto& i : map) { i.first; i.second; }

Would it be possible to have a free function (or member) that would allow me to do something like:
for(auto& i : as_map(jsonObj)) {...}
such that the iterator exposed here works more like map::iterator (.first,.second) than the current iterator type (.key,.value).

[nlohmann]

I did not find a way to realize this without any syntactic adjustment, but you can do this:

json j = {{"A", 1}, {"B", 2}};

for (auto i : json::iterator_wrapper(j))
{
  auto x = i.key(); // x = "A", x = "B"
  auto y = i.value(); // y = 1, y = 2
}

See https://github.com/nlohmann/json/blob/develop/test/src/unit-iterator_wrapper.cpp for more examples.

If you have any idea to realize this without a wrapper, please let me know!

[jaredgrubb]

I think it will require:

• a wrapper function
• a new iterator type (whose value type is pair<string,json> instead of key/value).

Personally, I really dont like the key/value functions and would rather just use first/second ... and this is the minimal approach I can think of. What do you think? I'm happy to start working on a patch, but if you think it's a non-starter, then I'll skip it.

[nlohmann]

So basically the syntax of the loop would be the same, but you'd rather write i.first than i.key()?

[jaredgrubb]

My goal is to make range-for over a json-object easy, which really doesnt work right now (well without your "iterator_wrapper" suggestion, which is not very discoverable). I personally prefer .first/.second because I want it to feel like a map, but that's bikeshedding.

Honestly, none of these sound like awesome solutions. Another option is to use get_ref to get the internal STL map object, but that's even worse to type.

[nlohmann]

So again - if there would be a better name for the iteration_wrapper (maybe also a more prominent documentation), and additional first/second members - would you see this as an improvement? Any name suggestions?

[nlohmann]

Any opinions on this?

[nlohmann]

I once again shall try to get another opinion on how to name the iteration_wrapper...

[jaredgrubb]

make_iterator(json&)?

[jaredgrubb]

make_object_iterator(json&)?

[nlohmann]

I'm still not too happy about the namings. In any case, does adding first/second as aliases for key() and value() make sense?

[holocronweaver]

I would rather have a member function than a free function for sake of reading and typing. Something like for (auto& i : j.range()) over for (auto& i : json::range(j)).
key() and value() are good enough for me, but aliases will probably ease in new users.
❤️ this library BTW.

[Rikorose]

The aliases could be added easily. Member variables with key and value aka first and second are more difficult though.

/*!
@copydoc iterator_wrapper(reference)
*/
static iteration_proxy<iterator> range(reference cont)
{
    return iteration_proxy<iterator>(cont);
}

/*!
@copydoc iterator_wrapper(reference)
*/
static iteration_proxy<const_iterator> range(const_reference cont)
{
    return iteration_proxy<const_iterator>(cont);
}

/*!
@copydoc iterator_wrapper(reference)
*/
iteration_proxy<iterator> range()
{
    return iteration_proxy<iterator>(*this);
}

/*!
@copydoc iterator_wrapper(reference)
*/
iteration_proxy<const_iterator> range() const
{
    return iteration_proxy<const_iterator>(*this);
}

[nlohmann]

The first member as alias for key() is not a problem. But I currently cannot find a way to get the second member to work as alias for value(). This is my current approach which fails when trying to compile the unit tests: f98d78b.

Any help is greatly appreciated!

[Type1J]

That solution makes you pay the copy cost of the key string even if you were just going to call .key(), which wouldn't use it. I think I know a better way. I'll make a PR for it.

[nlohmann]

@Type1J, your solution is much better than mine. I totally forgot about implicit conversions. Once Travis completes, I shall merge #578.

[nlohmann]

I need to update the documentation, so I reopened the issue.

[nlohmann]

There is still an issue in the code, see #578.

[nlohmann]

The test cases succeed now, but only because they just use operator== and operator!= which were added in #579. However, first and second are not fully behaving like key() and value(), respectively.

Consider this example:

#include "json.hpp"

using json = nlohmann::json;

int main()
{
    json j_object = {{"one", 1}, {"two", 2}};

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

It does not compile with Clang or GCC:

examples/iterator_wrapper.cpp:14:30: error: invalid operands to binary
      expression ('basic_ostream<char, std::__1::char_traits<char> >' and
      'iterator_key_property<nlohmann::basic_json<std::map, std::vector,
      std::__1::basic_string<char>, bool, long long, unsigned long long, double,
      std::allocator,
      adl_serializer>::iteration_proxy<nlohmann::basic_json<std::map,
      std::vector, std::__1::basic_string<char>, bool, long long, unsigned long
      long, double, std::allocator,
      adl_serializer>::iter_impl<nlohmann::basic_json<std::map, std::vector,
      std::__1::basic_string<char>, bool, long long, unsigned long long, double,
      std::allocator, adl_serializer> > >::iteration_proxy_internal>')
        std::cout << "key: " << x.first << ", value: " << x.second << '\n';
        ~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~

I would like to be able to have first and second be a drop-in replacement for key() and value(), because errors like the one above are confusing and hard to explain.

[Type1J]

Maybe we should take it out. It's strange that the implicit conversion works for some, but not all compilers.

[nlohmann]

I reverted the last PRs. It seems we need a new idea or we need to close this as "won't fix".

[Type1J]

I'm having a change of heart on this one. I think "won't fix" is the right course of action. Using .key() and .value() vs .first and .second indicate better that something is being done to fetch what you requested. There could be a conversion to std::pair<key_type, value_type>, which might let you write something like

for (std::pair<std::string, json> p : a_json_object)
{
   std::cout << p.first << ": " << p.second << std::endl;
}

, but it would require you to use an explicit std::pair type. Note also that it's a value type, not a reference, so you're paying a copy cost.

I think just using .key() and .value() may be best.