Get key and value using items()? #400

sndth · 2024-09-22T20:27:06Z

sndth
Sep 22, 2024

Hi! From what I saw, there is only one way to get unknown key with value:

for (auto i = root.begin(); i != root.end(); ++i) {
  std::cout << i.key() << '\n';
  std::cout << i.value() << '\n';
}

for example, in the json library we have items function and we can use:

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

or:

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

It would be good idea to implement items function to stop using old C++ loop!

fktn-k · 2024-09-23T05:01:07Z

fktn-k
Sep 23, 2024
Maintainer

@sndth
Thanks for your suggestion! Actually, I've had exactly the same issue when implementing the serializer which needs to traverse a mapping without knowing key values here...
So I really like json's items function and its usage, and agree with your suggestion! (Of course, just a similar feature since data structure differs between JSON and YAML)

Note that the new function won't work for sequences since the library throws an exception on calling the key function with sequences just as the current implementation:

// This is an example using the current implementation
for (auto itr = seq.begin(); itr != seq.end(); ++itr) {
    std::cout << itr.key() /*exception thrown here*/ << std::endl;
}

// will work as suggested with range-based for
for (auto& el : seq.items()) {
    std::cout << el << std::endl;
}
// but the above is the same as
for (auto& el : seq) {
    std::cout << el << std::endl;
}

// (if structured bindings get supported...)
// exception thrown at the `for...` line in the same way as the first one, even with `auto& [val]`,
// but is it necessary or even possible with dynamically sized array...?
for (auto& [_, val] : seq.items()) {
    std::cout << val << std::endl;
}

Given the above limitation, would it be better to name the function as something like map_items...?
I'd like to have your thoughts on it or anything you notice.

0 replies

sndth · 2024-09-23T13:45:44Z

sndth
Sep 23, 2024
Author

Will it be possible to detect the type of the element (variable) inside the items() function (mapping/sequence) then return valid values? If not, of course, new map_items() function could be created!

Can you also provide the YAML file to get the exceptions you got? Because I cannot reproduce these. Thank you!

0 replies

fktn-k · 2024-09-23T16:14:16Z

fktn-k
Sep 23, 2024
Maintainer

@sndth

Will it be possible to detect the type of the element (variable) inside the items() function (mapping/sequence) then return valid values?

That is possible and that's how the current implementation (like the begin()/end() functions) works if I understand your question correctly.
But the problem is, operator*() is called inside range-based for loops.
The operator currently returns

the value node of the current key-value pair when the "element" is a mapping node,
the value node at the current index when the "element" is a sequence node.

That is why we cannot get keys using range-based for loops now. To fix it, I guess we have two options.

Change the current iterator implementation of the operator*() to return the iterator itself so the key()/value() functions can be called in a range-based for loop. But that would rause another issue, that is, we cannot use range-based for loops for sequences as before.
Create another type of iterator which does the the above and leave the current iterator as-is (this is basically the way the json library took). But this solution seems asymmetric.

Frankly, I can't yet decide which one is better.

Can you also provide the YAML file to get the exceptions you got? Because I cannot reproduce these.

Try this YAML. (GitHub doesn't accept YAML files, but just three lines. Please copy and paste them...)

- foo
- bar
- baz

And my cpp file is:

#include <iostream>
#include <fstream>
#include <fkYAML/node.hpp>

int main() {
    std::ifstream ifs("./sample.yml");
    fkyaml::node n = fkyaml::node::deserialize(ifs);

    try {
        for (auto itr = n.begin(); itr != n.end(); ++itr) {
            std::cout << itr.key() << ": " << itr.value() << std::endl;
        }
    } catch (const fkyaml::exception& e) {
        std::cerr << e.what() << std::endl;
    }

    return 0;
}

You should see the following error log because the key() function is called against a sequence node.

Cannot retrieve key from non-mapping iterators.

0 replies

sndth · 2024-09-23T20:12:36Z

sndth
Sep 23, 2024
Author

Oh, I guess I saw. When you try to get key from array using nlohmann json, you will get item index:

[
  "abc",
  "def"
]

#include <fstream>
#include <iostream>
#include <nlohmann/json.hpp>

int
main()
{
  std::ifstream ifs("./sample.json");
  nlohmann::json j = nlohmann::json::parse(ifs);

  // result:
  //   "abc"
  //   "def"
  for (auto itr : j) {
    std::cout << itr << std::endl;
  }

  // result:
  //   0
  //   "abc"
  //   1
  //   "def"
  for (auto itr : j.items()) {
    std::cout << itr.key() << std::endl;
    std::cout << itr.value() << std::endl;
  }
}

and right - we can't do this currently.
So maybe the second solution would be enough for now?

0 replies

fktn-k · 2024-09-24T10:18:46Z

fktn-k
Sep 24, 2024
Maintainer

@sndth
Thanks for sharing the investigation!
Yes, we can't do the same (and I think don't have to, since YAML and JSON have similar but different data structures from each other).

Let's wrap up the discussion.
We'll implement a new feature so we can get keys without using the legacy style for loops. The implementation would be something similar to nlohmann json's items() function but different in that the new feature limits its scope to only mappings. So the function name will be changed to map_items() or something. Furthermore, the added feature supports structured bindings if this library is built with a compiler which supports C++17 features.

If I miss or misunderstand anything, please let me know. Also, I can implement the feature if you're not planning to do so.

0 replies

sndth · 2024-09-25T14:22:05Z

sndth
Sep 25, 2024
Author

Hello! I tried to implement map_items function but this caused me some problems, mainly due to the iterator class, which I can't understand (too much code haha).

I tried to use many methods, e.g. vector:

std::vector<iterator> map_items() {
  return {begin(), end()};
}

but this crashing (cannot dereference end map/set iterator) and showing only: a b from:

abc:
  a: b
  b: c
  c: d
  e: f

for (auto itr : n["abc"].map_items()) {
  std::cout << itr.key() << ' ' << itr.value() << std::endl;
}

2 replies

fktn-k Sep 25, 2024
Maintainer

@sndth
Thanks for the try! I guess some information needs to be shared (especially about the iterator class).

First of all, let me share a brief information on the iterator class. It's basically a wrapper class which contains both mapping and sequence iterators to provide commmon interfaces for accessing inner node elements and changes its behaviors depending on its actual iterator type (the value of m_inner_iterator_type): If the type is iterator_t::MAPPING, then an iterator class instance behaves as a mapping node iterator; if the type is iterator_t::SEQUENCE, on the other hand, it behaves as a sequence node iterator. Although there seems a lot of lines of codes, most of them are actually kind of boilerplates which need to be implemented so the iterator class works as many other iterator classes while providing common interfaces for accessing inner node elements at the same time. That said, for now, let's ignore 1) functions and operators except key(), value() and operator*() and 2) the case where the iterator behaves as a sequence node iterator because it's not relevant. The key() and value() functions, as their names imply, return a key node and a value node which the current mapping iterator refers to, respectively. The operator*() works just as the value() function if the actual iterator is a mapping node iterator.

Secondly, any iterator which are returned from an end() function shouldn't be used for dereference. For such an iterator typically refers to some inaccessible memory region (imaging the consequence of dereferencing nullptr). That's the crash you faced when you tried to dereference the second element (which is an iterator returned from the end() function) of the returned vector from the map_item() function. The implementation must instead return some object (not necessarily a vector) which returns an iterator equivalent to the returned iterator from the node's end() function like:

// warnning: This is just a pseudo code for explanation. It won't work.
std::vector<iterator> map_items() {
  return {begin(), begin() + 1, begin() + 2, ... , end()};
}

As you might notice, the above pseudo code has some issues:

A redundant traversal through a target mapping node is required within the map_items() function.
Heap allocation (using vector) is required because the actual size cannot be known at compile time. Heap allocation should be avoided when possible.

To fix the redundancy, what about creating a class which only have the iterators returned from the node's begin() and end(). It's difficult for me to explain it by words, so I'll share some code. This is not elaborated enough and some more implementation should be necessary, but hopefully sufficient for sharing my idea:

// the name is inspired from std::range but can be a different name
class map_range {
public:
    // this can be a separate class.
    // this class works to delay dereferencing actual `iterator` object.
    class iterator_wrap {
    public:
        iterator_wrap(iterator itr)
            : m_inner_itr(itr) {
        }

        // for the first dereference in a range-based for loop.
        iterator operator*() {
            return m_inner_itr;
        }

        // for incrementation in range based for loops
        iterator_wrap& operator++() {
            ++m_inner_itr;
            return *this;
        }

    private:
        iterator m_inner_itr {};
    };

    map_range(iterator begin, iterator end)
        : m_begin(begin),
          m_end(end) {
    }

    // range based for loops requires begin()/end()

    iterator_wrap begin() {
        return {m_begin};
    }

    iterator_wrap end() {
        return {m_end};
    }

private:
    iterator m_begin {};
    iterator m_end {};
};

The above code would be more understandable if you read how a compiler would generate code from range based for loops which is explained here.

Sorry for the too long reply. Feel free to ask anything which is unclear or needs more explanation!

fktn-k Jan 7, 2025
Maintainer

@sndth
The PR #458 has added the map_items() function as a new basic_node class API, which allows accessing mapping keys and values in range based for loops.
Sorry for it took too long for that feature to be available after the discussion.

Get key and value using items()? #400

Uh oh!

Uh oh!

sndth Sep 22, 2024

Replies: 6 comments · 2 replies

Uh oh!

fktn-k Sep 23, 2024 Maintainer

Uh oh!

Uh oh!

sndth Sep 23, 2024 Author

Uh oh!

fktn-k Sep 23, 2024 Maintainer

Uh oh!

Uh oh!

sndth Sep 23, 2024 Author

Uh oh!

Uh oh!

fktn-k Sep 24, 2024 Maintainer

Uh oh!

Uh oh!

sndth Sep 25, 2024 Author

Uh oh!

fktn-k Sep 25, 2024 Maintainer

Uh oh!

fktn-k Jan 7, 2025 Maintainer

sndth
Sep 22, 2024

Replies: 6 comments 2 replies

fktn-k
Sep 23, 2024
Maintainer

sndth
Sep 23, 2024
Author

fktn-k
Sep 23, 2024
Maintainer

sndth
Sep 23, 2024
Author

fktn-k
Sep 24, 2024
Maintainer

sndth
Sep 25, 2024
Author

fktn-k Sep 25, 2024
Maintainer

fktn-k Jan 7, 2025
Maintainer