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

json_object_merge does not merge #1

Open
ghost opened this issue Aug 29, 2014 · 4 comments
Open

json_object_merge does not merge #1

ghost opened this issue Aug 29, 2014 · 4 comments

Comments

@ghost
Copy link

ghost commented Aug 29, 2014

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#include "json.h"
#include "json-builder.h"

#define R(x...) #x

int main()
{
    const char *json1 = R({"x": 1, "y": 2});
    const char *json2 = R({"y": 3, "z": 4});

    char error[128];
    json_settings settings = {};
    settings.value_extra = json_builder_extra;
    json_value *obj1 = json_parse_ex(&settings, json1, strlen(json1), error);
    json_value *obj2 = json_parse_ex(&settings, json2, strlen(json1), error);

    json_object_merge(obj1, obj2);

    char *buf = malloc(json_measure(obj1));
    json_serialize(buf, obj1);

    printf("buf: `%s'\n", buf);
    return 0;
}
buf: `, "z": 4{ "y": 3, "z": 4 }'

That's odd, isn't it? I think, I was supposed to get {"x": 1, "y": 3, "z": 4"} here. Maybe the function should be rewritten completely using the recursive scheme. I suppose, it must be good to have an ability to merge e.g.

{"arr": [{"id": "1", "data": "A"}, {"id": "2", "data": "B"}]} <+>
{"arr": [{"id": "1", "extra": "@"}, {"id": "2", "data": "Z"}, {"id": "C"}]} ==
{"arr": [{"id": "1", "data": "A", "extra": "@"}, {"id": "2", "data": "Z"}, {"id": "3"}]}

I'll try to take a look on the sources in a couple of days and figure it out.

@jamesamcl
Copy link
Collaborator

Yep that's not right at all - merge must be producing a malformed object to get that output from serialize.

I'll check it out this evening.

@jamesamcl
Copy link
Collaborator

Currently, the expected output from merge for that example would be {"x": 1, "y": 2, "y": 3, "z": 4}. Which is obviously not ideal, but it's O(n). I would also be interested in being able to get {"x": 1, "y": 2, "z": 4}, but that would be O(n^2) because it would have to search through the existing keys for each key added.

Perhaps I could rename the existing json_object_merge to json_object_extend (because it basically does the same thing as jQuery's extend), and add a flag to indicate whether to allow duplicate keys in the resulting object or (more expensively) replace any existing properties. Then a recursive, deep json_object_merge could be added with the behaviour you describe.

@jamesamcl
Copy link
Collaborator

As of e766b23, the output is { "x": 1, "y": 2, "y": 3, "z": 4 }. Tomorrow I'll have a look at renaming this to extend and adding the flag to allow/disallow duplicate keys, and then a real merge function can be added at some point.

I just noticed in your example: it doesn't matter in this case because the JSON snippets are the same length, but both of your json_parse_ex calls use strlen(json1). Cool trick with the R(...) macro by the way.

@ghost
Copy link
Author

ghost commented Sep 1, 2014

Oh, that's great news! Thank you. Yep, this is a dummy copy-n-paste error, thanks for noticing.

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

No branches or pull requests

1 participant