search

dropbox / json11

A tiny JSON library for C++11.
MIT License
2.54k stars 613 forks source link

has_shape() return true for NUL type only if element actually exists (fix #132) #133

Closed jbhelm closed 4 years ago

jbhelm commented 5 years ago

Fixes issue #132

jbhelm commented 5 years ago

Note the alternative to checking for static_null() as implemented in the PR is to just stop using operator[] and do something more like:

const auto& obj_items = object_items();
for (auto & item : types) {
    const auto it = obj_items.find(item.first);
    if (it == obj_items.cend() || it->second.type() != item.second) {
...
jbhelm commented 5 years ago

I agree with your points about the use of static_null() (which is why I posted the alternative above) -- I was attempting to continue use of operator[] in has_shape() to keep with its existing implementation. I've updated the PR to the version posted above.

I understand the default-value behavior of the library when accessing fields, however IMHO has_shape() has a different purpose. The object { "bar": 123 } definitely does not "have the shape of" {"foo", json11::Json::NUL }. However I understand if you don't want to incorporated this PR, but I'll leave it in my fork in case anyone else needs to distinguish between default-null valued and non-existent fields.

artwyman commented 5 years ago

Approved. This is a behavior change, but I feel it's unlikely that anyone actually wanted or depended on the old behavior.

@jbhelm I can't merge this until you sign the CLA here: https://opensource.dropbox.com/cla/

artwyman commented 4 years ago

Looks like CLA was signed on Apr 11, but I didn't notice because of a typo in the GitHub username. Will merge.