algesten / jsondiff

Structural JSON diff/patch tool.
Other
66 stars 27 forks source link

More details for readme #15

Closed nziehn closed 11 years ago

nziehn commented 11 years ago

Hey,

I dont really understand the part in the readme, when you talk about the instruction order.

I think defining instructions that need a specific order in a json dictionary object ( key/value pair object) is not really good idea. I wanted to adopt your protocol in another programming language and the array of keys is not sorted and therefore the same order is not guarantied. ( In Java I'd use a HashMap .. ).

But maybe this is not an issue because I misunderstood the readme. In this case you could maybe elaborate a little bit more on this point in the readme.

Thanks anyways :)

Cheers, Nils

algesten commented 11 years ago

Hm. Perhaps I need to clarify the readme. As you point out, an associative array or a hashmap doesn't have a defined key order. However when patching the json object I need to define an order. Consider this:

Orig:

{
    a: [0,1]
}

Patch:

{
    a[1]: 42, // SET
    a[+1]: 99 // INSERT
}

In the patch object we can't guarantee the order of the keys. However depending on the order of the operations, the result would be completely different:

Hence, I have defined an order the I pick out the mutation operations from the map – which doesn't mean the key order in the map is respected (it isn't).

algesten commented 11 years ago

And looking at the problem again, I realise I haven't documented the whole truth. In addition to the instruction order MERGE, SET, INSERT, DELETE. Each individual instruction is further sorted so that INSERT are done in ascending index order and DELETE in descending index order.

Patch:

{
    a[+2]: 42,
    a[+1]: 99
}

Is sorted so that a[+1] happens before [a+2]. However in delete the reverse is true:

Patch:

{
    -a[1]: 99,
    -a[2]: 42
}

So that -a[2] happens before -a[1].

The relevant method is compareTo() in JsonPatch around row 420.

nziehn commented 11 years ago

Ah ok! Thats the part I was missing. Thank you! You should add that to the readme ;)