Add emptyObject() and emptyArray() functions to std/json
Following this thread on the forums: https://forum.dlang.org/thread/[email protected] I also discovered that it's a bit unwieldy to make an empty object, and usually I just resort to the following:
JSONValue j = JSONValue(string[string].init);
But this is not very clear for beginners and is also a bit of a mouthful. I propose with this PR to change it to this:
JSONValue j = JSONValue.emptyObject();
In addition, I have added an equivalent function to initialize an empty array node:
JSONValue j = JSONValue.emptyArray();
Thanks for your pull request and interest in making D better, @andrewlalis! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:
- My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
- My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
- I have provided a detailed rationale explaining my changes
- New or modified functions have Ddoc comments (with
Params:andReturns:)
Please see CONTRIBUTING.md for more information.
If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.
Bugzilla references
Your PR doesn't reference any Bugzilla issue.
If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub run digger -- build "master + phobos#8559"
What's wrong with JSONValue(string[string].init);? It's the implementation in this PR anyway, and doesn't need to be in Phobos to be used:
import std.json;
void main() {
assert(emptyObject.type == JSONType.object);
}
auto emptyObject() @safe pure {
return JSONValue(string[string].init);
}
Maybe this is more of a documentation bug?
What's wrong with
JSONValue(string[string].init);? It's the implementation in this PR anyway, and doesn't need to be in Phobos to be used:
I would argue that emptyObject() is necessary because it's something which every single other JSON implementation I've used in every other language has provided in one form or another, so it seemed a bit of a hassle to me that I needed to be acutely aware of the underlying implementation of std.json in order to do things which feel more "out-of-the-box" with other implementations.
As you said, this trick for initializing empty JSON objects and arrays could be better documented, but ultimately to me, it still feels like a bit of a trick when writing it, and when done a lot, makes things just a bit less readable. Perhaps that is just a contrast of my opinion against yours though.
I would say that the root of the issue, really, is that D doesn't have syntax for an empty AA literal.
For arrays, we have [], and we can add a constructor which accepts [] (type void[], checking that it's empty at runtime) and returns an empty JSON array.
However, {} is a function literal, not an AA.
Though... we could do this...
this(void function() pure f) { this((string[string]).init) }
{} is a function literal for a pure function which returns void. A pure function which returns void is meaningless: its invocation can have no effect and can be omitted entirely. So, if such a function is passed to the constructor of a JSONValue, it could be assumed to be {}, which looks like an empty AA literal (in D, and in JSON/JavaScript).
This is more of an idle observation though; we probably shouldn't actually add such a hack. :)
A couple of notes:
-
JsonValue(string[string].init)requires a runtime loop over an empty AA. The current implementation (if it has been changed from that) is better, as it's a simple assignment. - Both
emptyObjectandemptyArraycan be 100% computed at compile time, so why not make them enums?
What @schveiguy said.
Sorry that it took so long... went on holiday and started a new job. But anyways, I've changed to make emptyObject and emptyArray defined via enums, but @schveiguy, I'm not sure exactly what you mean by your first point:
The current implementation (if it has been changed from that) is better, as it's a simple assignment. I'm not too familiar with what goes on behind the scenes with associative arrays, but what are you referring to as the "current implementation"?
@schveiguy @atilaneves sorry for the pings, but I would appreciate if you could take another look at my PR at some point, and let me know if there's anything else needed, or if it should be discarded.
I rebased the branch of the PR to restart the CI.