Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add nested object capability to pointers #323

Closed
clwill opened this issue Oct 7, 2016 · 14 comments
Closed

Add nested object capability to pointers #323

clwill opened this issue Oct 7, 2016 · 14 comments

Comments

@clwill
Copy link

clwill commented Oct 7, 2016

You currently can create a new object with

j["this"]["that"] = 27;

and if this and/or that don't exist, it's all good, it creates this as an object and puts that in it.

But if you use pointers (e.g. "/this/that") it throws an exception if this doesn't exist.

For example:

j.at("/this/that"_json_pointer) = 27;

would be nice.

@nlohmann
Copy link
Owner

nlohmann commented Oct 7, 2016

I can confirm your observation - however, at is supposed to throw when applied to a nonexisting key.

Using j["/this/that"_json_pointer] = 27; still throws - this is a bug. What currently works is j["/this"_json_pointer] = 27;. That is, the operator[] for JSON Pointers shall be extended to work with arbitrary nested pointers even if parts are not existing yet.

@clwill
Copy link
Author

clwill commented Oct 7, 2016

Of course, doh!

Thanks, I'm dealing with a complex structure and the ability to not have to parse/check the existence of each level is quite helpful.

@clwill
Copy link
Author

clwill commented Oct 7, 2016

Also, don't know if you saw my comment on #278, but there is a parallel issue with value. I think that

j.value("/this/that"_json_pointer, 27);

should return 27 if either this or that are not present, not throw on a missing this.

Thx.

@gregmarr
Copy link
Contributor

gregmarr commented Oct 7, 2016

Presumably it's "/this/that"_json_pointer that's throwing, not .value() itself.
I think I misread that previous comment as _json_pointer was the part throwing.
Never mind.

@nlohmann
Copy link
Owner

nlohmann commented Oct 7, 2016

Thanks for noting. The reason is that both functions use the same function inside the json_pointer class.

@clwill
Copy link
Author

clwill commented Oct 7, 2016

Got it. Didn't dive inside, it was late :)

nlohmann added a commit that referenced this issue Oct 8, 2016
@nlohmann nlohmann self-assigned this Oct 8, 2016
@nlohmann
Copy link
Owner

nlohmann commented Oct 8, 2016

The bug was that there was no code to cope when a JSON pointer was used on a null value. I'm on it.

nlohmann added a commit that referenced this issue Oct 10, 2016
@nlohmann
Copy link
Owner

I fixed the issue with operator[](const json_pointer&) and now shall have a look at your comment regarding value(const json_pointer&, ValueType).

@nlohmann
Copy link
Owner

Let's discuss the semantics of value with JSON pointers in #323.

@clwill - Can you please have a look at branch https://github.com/nlohmann/json/tree/feature/issue323 if this fixes the issue?

@clwill
Copy link
Author

clwill commented Oct 10, 2016

I've done some playing with the branch above, and it seems to work as expected. I like it!

@clwill
Copy link
Author

clwill commented Oct 10, 2016

BTW, just so you can understand my use case... I have a generic configuration module that puts all the other modules' defaults into one master .json file. The value setting function used to be:

template<typename T>
void setValue(const std::string keyName, T val) {
    jsonMaster[moduleName][keyName] = val;
}

Which worked and was OK. But with this update, I can do:

    jsonMaster[json::json_pointer("/" + moduleName + "/" + keyName)] = val;

The modules setting a value can now not only pass in a keyName of, say "this" but they can also make their own nested values of, say "this/that". This is very helpful.

Thank you.

@nlohmann
Copy link
Owner

Great. I shall merge the branch by the end of the week. However, I have no release date for 2.0.6 yet.

@clwill
Copy link
Author

clwill commented Oct 11, 2016

Thanks! Timing's OK, I can live with the [this][that] approach for the time being.

@nlohmann
Copy link
Owner

Merged the feature branch to develop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants