certik / yaml-cpp

Automatically exported from code.google.com/p/yaml-cpp
MIT License
0 stars 0 forks source link

Better error messages when dereferencing (e.g.) a seq iterator as if it were a map iterator #191

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
When parsing a yaml file such as

---
title: Home
items:
  - Key:
      SubKeyOne: [] #sequence of integers, in the order they are dependent
      SubKeyTwo:
...

Load the file(successful), std::cout << (file appears proper), then try
for(auto it=node["items"].begin();it!=node["items"].end();++it)
{
   std::cout << it->first << std::endl;
}

calls to it->first blow up [TypedBadConversion] as it->type evaluates as Null.  
This should result in a Scalar (string) of "Key".

If I backup one step and ask for 
std::cout << it
then I will see
Key:
   SubKeyOne: [] #sequence of integers, in the order they are dependent
   SubKeyTwo:
which is correct!
Am I applying the iterator followed by ->first and ->second technique found in 
the Tutorial page incorrectly?

Original issue reported on code.google.com by JohnMich...@gmail.com on 11 Feb 2013 at 4:01

GoogleCodeExporter commented 9 years ago
I should add that skipping the cout overloads and directly trying
std::string temp = it->first.as<std::string>();
also throws this TypedBadConversion

Original comment by JohnMich...@gmail.com on 11 Feb 2013 at 4:04

GoogleCodeExporter commented 9 years ago
In your YAML file, node["items"] is a sequence, so when you iterate through, 
the iterators point to single entries (the entries of the sequence), not 
key/value pairs.

In other words:

for(auto it=node["items"].begin();it!=node["items"].end();++it) {
   YAML::Node entry = *it;
   // now, you can either iterate through the map:
   for(auto mapIt=entry.begin();mapIt!=entry.end();++mapIt) {
      std::cout << mapIt->first << "\n";
   }
   // or you can access by key:
   std::cout << entry["Key"] << "\n";
}

I will interpret this issue as a request for better a error message when you 
use the wrong dereferencing on an iterator :)

Original comment by jbe...@gmail.com on 11 Feb 2013 at 4:13

GoogleCodeExporter commented 9 years ago
OK.  This seems counter-intuitive to me as both types of collections are 
defined as being composed of Nodes 
(http://www.yaml.org/spec/1.2/spec.html#id2764044).  Is there an implementation 
reason why the iterators can't act like Nodes by default?

Original comment by JohnMich...@gmail.com on 11 Feb 2013 at 4:29

GoogleCodeExporter commented 9 years ago
Ok, after reading the specification very carefully I believe I see the issue.  
Mapping nodes are defined as an unordered set of key/value pairs of any size.  
Hence single key/value nodes are not immediately distinguishable from larger 
maps.

I would propose a convenience method, although I can understand if one might 
have style objections.
Define first and second for YAML::Node.  Those methods would look like

YAML::Node first()
if(node.Type() != NodeType::Map)
  throw NodeNotMap
else
  return keys[0];

YAML::Node second()
if(node.Type() != NodeType::Map)
  throw NodeNotMap
else
  return values[0];

In the case of single key/value nodes the return values are intuitive.  In the 
case of maps > 1 the function would return the first key or value stored in 
memory.  This is not guaranteed to be ordered (and hence it is really only 
useful for maps with 1 k/v pair), but this keeps it fast and this would be 
noted in the documentation.  One example use case of this would be putting maps 
inside a sequence in order to create an ordered-map.  This method of creating 
ordered-maps is 1.2 compliant and thus isn't dependent on the parser.  
Offtopic: it's enhanced in appearance by the map-in-sequence shortcut: 
http://www.yaml.org/YAML_for_ruby.html#mapping-in-sequence_shortcut

Original comment by JohnMich...@gmail.com on 11 Feb 2013 at 5:11

GoogleCodeExporter commented 9 years ago
Yeah, I've thought about what an interface to access an ordered map would look 
like, but I'm not sure if this is the right thing to do. I'm not a huge fan of 
adding first() and second() methods on Node.

I will keep in mind the general question, thought. Thanks for the suggestion!

Original comment by jbe...@gmail.com on 11 Feb 2013 at 6:29

GoogleCodeExporter commented 9 years ago
Fixed, 
https://code.google.com/p/yaml-cpp/source/detail?r=09685b5f18dd422ed04c6cdd53d9e
931585491b8.

Original comment by jbe...@gmail.com on 13 Apr 2013 at 4:43