vivkin / gason

Lightweight and fast JSON parser for C++
MIT License
338 stars 51 forks source link

Iterator pointer semantics fix #18

Open duckie opened 9 years ago

duckie commented 9 years ago

Hello good sir

While writing a wrapper around gason for my company, I ran into a small problem. Here is a fix proposal.

API breakage here, but original API is erroneous.

The JsonIterator::operator* in gason does not implement correctly the pointer semantics. An iterator should be used in those ways (assuming v is a value containing an object).

for(auto it = begin(v); it != end(v); ++it)
  cout << it->key << endl;
for(auto node : v)
  cout << node.key << endl;

But in gason is used like this:

for(auto node : v)
  cout << node->key << endl;

This is not consistent with common behaviors of C++ iterators. This patch fixes it.

ChrisJefferson commented 9 years ago

While I actually agree with this patch in principle, I would hope it would wait until a major release, as it's going to break every piece of code using gason!

duckie commented 9 years ago

Yes it does. There is no problem into delaying it for a major release. Do you plan one ? Do you want me to request the pull into a release candidate branch ?

ChrisJefferson commented 9 years ago

Just to say, I'm just a random guy who read and commented on your patch, not @vivkin :)