Skip to content
This repository has been archived by the owner on Dec 1, 2023. It is now read-only.

Change Object underlying type from BTreeMap to HashMap #56

Closed
frewsxcv opened this issue Feb 15, 2015 · 9 comments
Closed

Change Object underlying type from BTreeMap to HashMap #56

frewsxcv opened this issue Feb 15, 2015 · 9 comments

Comments

@frewsxcv
Copy link
Contributor

Right now, an Object is pub type Object = BTreeMap<string::String, Json>;

According to the Rust docs:

Use a HashMap when:

    You want to associate arbitrary keys with an arbitrary value.
    You want a cache.
    You want a map, with no extra functionality.

Use a BTreeMap when:

    You're interested in what the smallest or largest key-value pair is.
    You want to find the largest or smallest key that is smaller or larger than something
    You want to be able to get all of the entries in order on-demand.
    You want a sorted map.

Based off of those distinctions, why should Object be a BTreeMap? From what I understand, we don't need it sorted.

@frewsxcv
Copy link
Contributor Author

I can make the appropriate changes if this is approved

frewsxcv added a commit to frewsxcv/rustc-serialize that referenced this issue Feb 16, 2015
Fixes rust-lang-deprecated#56

One side effect of this, is Json no longer derives PartialOrd since
HashMap doesn't derive it. That said, I don't really see why it should
derive it in the first place.

[breaking-change]
@alexcrichton
Copy link
Contributor

I find that a generic storage like JSON often requires the keys to be sorted at one point or another or at least vastly simplifies some form of operation over JSON. I do agree, however, that strictly speaking it may want to be a HashMap instead of a sorted tree.

Something like this, however, I think may be best left to the stabilization of this crate in the long run. I would hate for us to choose HashMap now only to go back later to BTreeMap, for example. Also, as one data point, I personally switched toml-rs from HashMap to BTreeMap as well.

@s-panferov
Copy link
Contributor

Personally I would prefer a collection, that could preserve an order of insertions. This is very convenient for things like rustless where you work with JSON API. When I read JSON output I prefer to see:

{ 
    "id": 100500,
    "name": "Cool thing"
}

instead of (what we have now):

{ 
    "name": "Cool thing",
    "id": 100500
}

Maybe we can rewrite json to support generic collection?

@frewsxcv
Copy link
Contributor Author

@s-panferov Do you prefer the first one because it's sorted alphabetically? If so, why not just change the JSON writer to sort keys before outputting?

@s-panferov
Copy link
Contributor

@frewsxcv I prefer the first one because the keys appear in the insertion order, not alphabetically.

@frewsxcv
Copy link
Contributor Author

Ah, I missed that in the first sentence of your original comment, sorry about that.

Forgive me if I sound defensive about this. I don't feel that strongly about this, but instinctively for something that is defined to be 'unordered', to me it makes sense to use an unordered data structure.

Personally I would prefer a collection, that could preserve an order of insertions.

This is not necessarily what everyone needs. If this is for debugging purposes, I personally would find more value in having the keys be sorted alphabetically when being displayed. I'm not in favor of changing the internal data structure solely for this specific use case.

Maybe we can rewrite json to support generic collection?

This might be an option, but as far as I know, there is no official Map collection trait yet, so this wouldn't be possible.

@agentgt
Copy link

agentgt commented Aug 4, 2015

Jackson (the Java JSON library) preserves order and treats JSON very similar to XML (a tree with elements in the order they appear). That is it has its own tree data structure. I will add it is an extremely mature JSON library and thus a good one to model.

I think the way Jackson does it is the correct approach even though the JSON spec does not really say anything about order.

Insertion order (or in document order) is particularly useful if one has both an XML and JSON representation (which is typical in REST APIs) and of course unit testing (which I suppose is the same point of debugging). But ordered JSON is also important for streaming purposes which Jackson also supports.

@dtolnay
Copy link
Contributor

dtolnay commented Feb 2, 2017

In serde_json we have a cfg that controls the representation - BTreeMap by default or LinkedHashMap if you need to preserve insertion order.

@alexcrichton
Copy link
Contributor

I'm going to close this now that this crate is deprecated in favor of serde. We're discontinuing feature development in rustc-serialize but will still continue to merge bug fixes if they arise.

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

Successfully merging a pull request may close this issue.

5 participants