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

return json objects from functions #1172

Closed
itodirel opened this issue Jul 25, 2018 · 15 comments
Closed

return json objects from functions #1172

itodirel opened this issue Jul 25, 2018 · 15 comments
Labels
kind: question solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@itodirel
Copy link

itodirel commented Jul 25, 2018

Given a function like json json_add(json& j, std::string name), which given a json object, adds another object to it and returns it, I expect that if I call:

json j;
json_add(j, "1");
json j2 = json_add(j, "2");
json_add(j2, "3");

The last call to json_add would add another object on an existing one created before, to create a hierarchy, but instead nothing happens, is there a way achieve this programatically?

json j = 
{
    { "filename", "file.cpp" }
};
json j2;
j2["prop"] = "1";

j["j2"] = j2;

j2["prop2"] = "2"; // after adding j2 to j, their reference is lost, so further updates to j2 does nothing

Expected:

{
  "filename": "file.cpp",
  "j2": {
    "prop": "1"
    "prop2": "2"
  }
}
@gregmarr
Copy link
Contributor

gregmarr commented Jul 25, 2018

j["j2"] = j2;

This stores a copy

j2["prop2"] = "2";

This modifies the original.

j["j2"]["prop2"] = "2";

This modifies the version in j.

@nlohmann nlohmann added kind: question solution: proposed fix a fix for the issue has been proposed and waits for confirmation labels Jul 25, 2018
@itodirel
Copy link
Author

itodirel commented Jul 25, 2018

@gregmarr that would not work, if you need to do it programatically, N-levels deep, which I need to :)

I understand the value semantics and the copies, but that doesn't mean that internally each "node" can't track each other. Without it, you can't build and patch hierarchies easily, and it's unnatural.

What I had to do is to make a pointer and find the element I want to patch using .at, and return it by reference.

@itodirel
Copy link
Author

the other thing, it would be cool if there was an add or append API that specifically returns a json reference of the element that was just added

@gregmarr
Copy link
Contributor

I'm having a hard time visualizing what it is that you're trying to do. Maybe this:

auto &nested = j["j2"];
nested = j2;
nested["prop2"] = 2;

@nlohmann nlohmann added state: needs more info the author of the issue needs to provide more details and removed solution: proposed fix a fix for the issue has been proposed and waits for confirmation labels Jul 26, 2018
@nlohmann
Copy link
Owner

I agree with @gregmarr in that I do not understand the issue yet. Could you try to describe your expected outcome and the difference to the actual behavior in more detail?

@itodirel
Copy link
Author

itodirel commented Jul 27, 2018

Here is an example:

json& add_node(json& node, std::string text)
{
    json new_node;
    new_node["node"]["text"] = text;
    node["nodes"].push_back(new_node);
    return new_node;
}

And the calling code, which creates a hierarchy but doesn't work because of the value semantics and copies:

json j;
add_node(j, "1");
add_node(j, "2");
add_node(j, "3");
json& node = add_node(j, "4");
add_node(node, "1");

I have to write to something like this, which does work, but is hacky:

json& add_node(json& node, std::string text)
{
    json new_node;
    new_node["node"]["text"] = text;
    node["nodes"].push_back(new_node);
    json::json_pointer p(std::string("/nodes/") + std::to_string(node["nodes"].size() - 1) + "/node");
    return node.at(p);
}

But then I eventually wrote it as, which is better:

json& add_node(json& node, std::string text)
{
    json new_node;
    new_node["node"]["text"] = text;
    node["nodes"].push_back(new_node);
    return node["nodes"].back()["node"];
}

I still expect an API that appends and returns a reference directly though, something like:

json& add_node(json& node, std::string text)
{
    json new_node;
    new_node["node"]["text"] = text;
    return node["nodes"].push_back(new_node);
}

@nlohmann
Copy link
Owner

As explanation: The API is the same as for a std::vector, so push_back is void.

@itodirel
Copy link
Author

itodirel commented Jul 27, 2018

personally, I don't think that's not sensible, and I think it's inconsistent to have a push_back API, on something that might not be an array, but when used becomes an array, at which some of the other APIs wouldn't work, it's very unintuitive

@nlohmann
Copy link
Owner

I don't understand: for arrays, push_back behaves just like std::vector::push_back. Only for null values we extended the behavior to allow for an implicit conversion to an arrays. Why is it unintuitive to return void?

@itodirel
Copy link
Author

itodirel commented Jul 27, 2018

not because it returns void, but because a json object has a vector API and non vector APIs on it, all together

@nlohmann
Copy link
Owner

I see. That's this library's way to deal with JSON's different value types. When you only see a json type, you don't know which type it is. What would be the alternative? Throwing?

@gregmarr
Copy link
Contributor

I still expect an API that appends and returns a reference directly though, something like:

json& add_node(json& node, std::string text)
{
    json new_node;
    new_node["node"]["text"] = text;
    return node["nodes"].push_back(new_node);
}

That is the equivalent of this:

json& add_node(json& node, std::string text)
{
    json new_node;
    new_node["node"]["text"] = text;
    node["nodes"].push_back(new_node);
    return node["nodes"].back();
}

or better

json& add_node(json& node, std::string text)
{
    json new_node;
    new_node["node"]["text"] = text;
    auto &nodes = node["nodes"];
    nodes.push_back(new_node);
    return nodes.back();
}

But is NOT the same as this:

But then I eventually wrote it as, which is better:

json& add_node(json& node, std::string text)
{
    json new_node;
    new_node["node"]["text"] = text;
    node["nodes"].push_back(new_node);
    return node["nodes"].back()["node"];
}

Other than using node["nodes"] more than once, this is basically correct, if you are expecting a JSON like this:

{ 
  "nodes":  [ 
    "node": { "text": "1" },
    "node": { "text": "2" },
    "node": { "text": "3" },
    "node": {
      "text": "4",
      "nodes": [ { "node": { "text": "1" } } ]
    }
  ]
}

I would say the best way to write this would be something like (untested, but the syntax should be like this):

json& add_node(json& node, std::string text)
{
    auto &nodes = node["nodes"];
    nodes.push_back({"node", {{"text", text}} });
    return nodes.back()["node"];
}

@nlohmann
Copy link
Owner

nlohmann commented Aug 4, 2018

@itodirel Can you comment on
#1172 (comment)?

@itodirel
Copy link
Author

itodirel commented Aug 7, 2018

I wrote something like that and it works for me, I'm not blocked, I just expected the node insertion to give me back the node I just inserted. Also thank you much for all your help and suggestions, it's very much appreciated.

@nlohmann nlohmann added solution: proposed fix a fix for the issue has been proposed and waits for confirmation and removed state: needs more info the author of the issue needs to provide more details labels Aug 8, 2018
@nlohmann
Copy link
Owner

nlohmann commented Aug 8, 2018

Thanks for checking back! We shall keep the push_back() function void as we have the back() function to return the last element.

@nlohmann nlohmann closed this as completed Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: question solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

3 participants