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

Nodes order lost due to HashMap usage #228

Closed
ikhomyshyn opened this issue May 6, 2016 · 18 comments
Closed

Nodes order lost due to HashMap usage #228

ikhomyshyn opened this issue May 6, 2016 · 18 comments

Comments

@ikhomyshyn
Copy link

I am converting JSON document to xml using following code (lib version 20160212):
String xml = XML.toString(new JSONObject(document), "Doc");

The problem is that HashMap used as map implementation in JSONObject, that breaks initial order of elements. LinkedHashMap would be better choice.

@erosb
Copy link
Contributor

erosb commented May 6, 2016

It has been reported & discussed many times already:
#210
#66
#43
#37

@ikhomyshyn
Copy link
Author

How would you suggest to produce valid xml that requires certain order of elements from JSON that already has properly ordered nodes (I know that JSON specification ignores order, but we leave in real world where order often matters)?

@erosb
Copy link
Contributor

erosb commented May 6, 2016

I don't know if any other JSON libraries do the job for you. But if your schema is not too complex then you may develop a hand-written converter, which creates the XML nodes in the right order.

Regardless usecases, the library won't support ordering. Even a working PR has been created already and it wasn't merged: #216 (comment) .

@ikhomyshyn
Copy link
Author

Thanks for your response.

P.S. IMHO: it is always better to give the ability to choose to the library clients (i.e. explicitly point that order matters) rather than refuse feature requests.

@johnjaylward
Copy link
Contributor

johnjaylward commented May 6, 2016

It's not a feature request. You are asking a bug to be placed into the library to suit your out-of-spec requirement.

If you really feel that XML output should support ordering the keys (which really the default alphabetical ordering is just arbitrary anyway), then you should implement it solely in the XML class and require no changes to JSONObject.

So far all the pull requests have focused on changing how JSONObject works, which is breaking the code from the specifications. However, if you can modify the XML class to do what you need, I don't think it would be an issue accepting that pull request.

One suggestion may be to allow the user to pass a comparator into the public static String toString(Object object, String tagName) method. Then they can specify whatever order they require through that.

@stleary
Copy link
Owner

stleary commented May 7, 2016

Thanks for the suggestion, but the design requires us to follow the spec - JSONObject contents are unordered.

@stleary stleary closed this as completed May 7, 2016
@ikhomyshyn
Copy link
Author

@johnjaylward, if you refer to ordering as a bug, then it is already present in the current version (HashMap orders entries by their hash codes and you can't guaranty that no one from lib users rely on that order).

If you really feel that XML output should support ordering the keys (which really the default alphabetical ordering is just arbitrary anyway), then you should implement it solely in the XML class and require no changes to JSONObject.

I agree that ordering might be implemented in the XML class, but usage of the comparator will be real pain in the neck for complex XSD. Passing XSD as parameter to public static String toString() seems the best option for me.

Current implementation of org.json.XML already break XML spec (it allows to transform unordered JSON object to ordered XML sequence) so maybe it is worth to introduce OrderedJSONObject for such spec-apostates as me?

@stolstov
Copy link

@stleary Is this implementation of the JSON object really produces unordered sequence? If each time you'd insert elements in the same order, and then iterate - you should get exactly same sequence each time (but different from the insertion order). Could you provide an option to preserve the insertion order?

@stleary
Copy link
Owner

stleary commented May 13, 2016

@stolstov This app has to follow the JSON spec, so JSONObjects are required to be unordered. Actually the implementation uses HashMap, so it is not random. However, you should not have expectations about the ordering as entries are added and removed, nor should you expect 2 different JSON implementations to produce identical orderings.

If ordering is important, feel free to fork the project and use a different internal collection. Or if you have control over the key-value pairs content, you can always use JSONObject.keySet() and sort the returned keys.

@stolstov
Copy link

@stleary It seems the JSON spec intent only to make sure that two objects that are different only by the order should be considered equal. It should not be that the parsed object must be scrambled and written in random order.

The keys are often put in the order for aesthetic reasons. A good example is to make a JSON that looks nice in the help page and in the real output. We don't need sorted keys, only to preserve the original order. LinkedHashMap works there.

@erosb
Copy link
Contributor

erosb commented May 13, 2016

Although I know @stleary has a solid opinion about this topic, I think adding an optional way for users to use LinkedHashMap as a storage for a JSONObject is a useful feature in some cases, and it does not violate the JSON spec: any client code that doesn't assume any ordering still works even if a given ordering exists. In other words if the client code doesn't assume any ordering then it doesn't matter if a given ordering is present. So if someone writes code which expects ordering, then the client code will be broken (assuming that a code not working on any JSONObject instances is broken) and not the library code itself. Since it seems to have valid practical usecases I think we can assume developers will be wise enough to decide when to use and when not to use the LinkedHashMap for internal representation.

From an other aspect I'm likely to think that ordered data structures are a subset of unordered data structures, instead of these terms forming distinct sets, since any operation that can be performed on an unordered set can also be performed on an ordered set. Adding ordering can be modeled as a specialization, or subtyping.

As a distant analogy, it is similar to the set of deterministic Turing machines being a subset of nondeterministic Turing machines.

@stolstov
Copy link

@johnjaylward
Copy link
Contributor

@stolstov Java does not offer a true random order map. The closest thing it offers built in is the hashmap. This library is designed to be simple and not have any external dependencies. Using a true random map would introduce either complexity or external dependency.

I'm not a fan of adding an ordered list into the library. It was originally removed back in 2011 or so by the original author, and requests to add it back in have been denied historically. If clients are not supposed to depend on order, I see no reason to specifically add an ordered implementation which increases the complexity of the code base (even minimally). Just because other libraries offer the feature is no reason to add it into this one.

As stated by @stleary, this is a reference implantation. If you need more features forking is always an option. In fact it's what I did to add in my own ordered lists for debugging purposes. Having my implementation in the main library is not something I think is appropriate though.

@stolstov
Copy link

stolstov commented May 16, 2016

@johnjaylward
The JSON.org page specifically mentions that different languages have different data structures to represent the collection of name/value pairs:
"JSON is built on two structures: A collection of name/value pairs. In various languages, this is realized as an object, record, struct, dictionary, hash table, keyed list, or associative array"

Now "An object is an unordered set of name/value pairs". I understand this statement that there should not be made any assumption about the order of the pairs in the JSON document, because different languages or implementations use arbitrary order. I don't believe that the org.JSON library would violate the spec by a particular choice of a container in Java, as the spec does not mentions this, only the data format. Nor does the spec mentions that the writer should change the input order.

If you'd provide an ordered container as an option, it would only make the library better in my opinion. Edit: I meant to say a container that preserves the input order (not the one that sorts lexicographically).

@ygopala
Copy link

ygopala commented Nov 1, 2016

Is the order of JSON Array [] maintained when we convert from JSON to XML?

@johnjaylward
Copy link
Contributor

@ygopala yes, why wouldn't it be?

@ygopala
Copy link

ygopala commented Nov 1, 2016

Thanks John!

Just wanted to know if the code maintains order for json arrays.

XML list to json array order is preserved as well?

@johnjaylward
Copy link
Contributor

it should be as lists/arrays in JSON are defined as ordered

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

6 participants