library-level docs for collections #17802

Gankra · 2014-10-05T17:51:57Z

Adds a high-level discussion of "what collection should you use for what", as well as some general discussion of correct/efficient usage of the capacity, iterator, and entry APIs.

Still building docs to confirm this renders right and the examples are good, but the content can be reviewed now.

Gankra · 2014-10-05T17:52:07Z

CC @aturon @steveklabnik

huonw · 2014-10-05T23:48:22Z

src/libcollections/lib.rs

+//! For optimal performance, collections will generally avoid *shrinking* themselves.
+//! If you believe that a collection will not soon contain any *more* elements, or
+//! just really need the memory, the `shrink_to_fit` method prompts the collection
+//! to *shrink* the backing array to the minimum size capable of holding its elements.


Is this emphasis needed? (There's a lot of *...* emphasis in this text?)

Haha, wow yeah. Good call.

Gankra · 2014-10-06T00:13:57Z

@huon Issues addressed.

steveklabnik · 2014-10-06T00:15:07Z

src/libcollections/lib.rs

+//! ### Use the `Set` variant of any of these `Map`s when:
+//! * You just want to remember which keys you've seen.
+//! * There is no meaningful value to associate with your keys.
+//! * You just want a set.


"Use a set when you want a set" isn't a particularly helpful guideine

Yeah it's a bit borderline, but it seemed... symmetric? My general pattern here is a gradient from "descriptive for newbies" to "what experts know", where "what experts know" is generally generic types like "it's the map" or "it's the set".

steveklabnik · 2014-10-06T00:18:53Z

Thank you, this is super great :)

Are the changes to lib.rs needed?

Gankra · 2014-10-06T00:19:55Z

@steveklabnik I'm not sure what you're referring to about changes to lib.rs?

steveklabnik · 2014-10-06T00:21:16Z

Sorry, btree/mod.rs

Gankra · 2014-10-06T00:23:56Z

@steveklabnik Yeah the examples won't work without it. It's just something that doing this revealed wasn't done. Figured I might as well toss it in.

huonw · 2014-10-06T00:23:57Z

src/libcollections/btree/mod.rs

@@ -15,6 +15,8 @@ pub use self::map::MoveEntries;
 pub use self::map::Keys;
 pub use self::map::Values;
 pub use self::map::Entry;
+pub use self::map::Occupied;


BTW, you can reexport multiple things at once pub use self::map::{Occupied, Vacant};.

Want me to change that? I was just following the pattern I saw elsewhere.

Gankra · 2014-10-06T01:40:51Z

Built and rendered.

Doesn't show up in the std re-export, which seems like a shame?

Gankra · 2014-10-06T02:16:29Z

Found a few errors or just weird statements. Also tossed in something about adapters and rev.

Gankra · 2014-10-06T03:00:42Z

Also I just remembered that I wrote this old thing back in the day when I first started poking at collections reform. I had always intended it to be paired with something like the contents of this PR, but I believe it was decided that it didn't have a good "place" in the docs. Anyone have any ideas?

aturon · 2014-10-06T03:01:15Z

src/libcollections/lib.rs

+//! # When Should You Use Which Collection?
+//!
+//! ### Use a `Vec` when:
+//! * You just want to store some items temporarily, to be processed or sent elsewhere later.


The first bullet, about temporary storage -- I'm curious what the justification is? I don't see how it argues for Vec over other collections, personally.

Pushing onto a Vec is surely the most efficient way to collect up a bunch of values, no? Similarly Vecs should iterate the fastest for extracting the values when you want them.

OK, I see what you have in mind. I think the "temporarily" bit threw me off -- the idea here seems to be that you just have a pile of items, where you don't care about the order or any notion of key, or equality, or priority. And yes, that argues for Vec :)

I'm not super thrilled about the current wording myself. If you have any suggestions I'm all-ears. :)

"You just want want an expanding sequence of items" (somewhat redundant with the next bullet, but maybe they can/should be merged)?

I feel like "great temp storage" and "great sequence" are distinct. Particularly the fact that order is significant in only one.

I think "temporary storage" has a clearer/more specific meaning for you than it does for me. Would be interested to see how others feel about this? (Honestly, we could probably just drop this bullet without losing much.)

aturon · 2014-10-06T03:20:42Z

@gankro Thanks for taking this on! This is great. A few scattered thoughts:

Are you still planning on including some kind of table with asymptotics for the main operations, as you see with the Scala and Qt docs? I think that'd be a useful addition to the informal list you gave.
I wanted a bit of an even-higher-level overview. Something like, collections include sequences, maps, and sets -- and for each case listing the concrete data structure names together with a very brief description of the algorithm meant for those who are familiar with data structures.
There's a nice overview of iterators here, but of course iter lives in a separate module (partly due to the organization of the std facade, where iter lives in libcore.) We should think a bit about the best way to distribute the documentation.

Anyway, nice work!

Gankra · 2014-10-06T03:26:11Z

@aturon

I'd like to wait for more API stabilization before tackling perf tables.
Super high-level seems doable. I'll poke at it.
I tried to talk about iterators only in the context of using collections, as iterators are a much more general tool, but incredibly important to using collections "right". Hopefully that makes this documentation appropriate.

aturon · 2014-10-06T03:27:48Z

@gankro Ok, those all seem like super-reasonable responses. Thanks!

Gankra · 2014-10-06T03:43:53Z

@aturon new commit addresses many of your concerns. Anything outstanding?

aturon · 2014-10-06T03:47:21Z

@gankro I'm happy to land what you've got now; always room for incremental improvements later on.

Gankra · 2014-10-06T03:48:24Z

@aturon squashed

aturon · 2014-10-06T05:24:11Z

@steveklabnik @huonw I'm happy with this -- are both of you?

steveklabnik · 2014-10-06T14:55:07Z

I'm fine with it.

huonw · 2014-10-07T03:01:29Z

Maybe there could be a link in the std::collections doc comment to this page, or else people probably won't see it.

It seems fine to me other than that.

Gankra · 2014-10-07T03:16:55Z

After some IRC discussion, I went with moving this to std::collections, and having collections direct the reader there.

Adds a high-level discussion of "what collection should you use for what", as well as some general discussion of correct/efficient usage of the capacity, iterator, and entry APIs. Still building docs to confirm this renders right and the examples are good, but the content can be reviewed now.

…r=Veykril fix: Surpress type mismatches in calls with mismatched arg counts These tend to get very noisy, hiding the actual problem.

Gankra mentioned this pull request Oct 5, 2014

provide detailed documentation of entry API #17609

Closed

huonw reviewed Oct 5, 2014
View reviewed changes

steveklabnik reviewed Oct 6, 2014
View reviewed changes

huonw reviewed Oct 6, 2014
View reviewed changes

Gankra force-pushed the collection-docs-redux branch from ec4de97 to d225903 Compare October 6, 2014 02:16

aturon reviewed Oct 6, 2014
View reviewed changes

Gankra force-pushed the collection-docs-redux branch from f6044f8 to 6ff1d78 Compare October 6, 2014 03:48

Gankra force-pushed the collection-docs-redux branch from 6ff1d78 to 2e633b3 Compare October 7, 2014 03:16

Gankra added 2 commits October 6, 2014 23:19

library-level docs for collections

8f4e855

add missing btree re-exports

1d6eda3

Gankra force-pushed the collection-docs-redux branch from 2e633b3 to 1d6eda3 Compare October 7, 2014 03:20

bors closed this Oct 7, 2014

bors merged commit 1d6eda3 into rust-lang:master Oct 7, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

library-level docs for collections #17802

library-level docs for collections #17802

Gankra commented Oct 5, 2014

Gankra commented Oct 5, 2014

huonw Oct 5, 2014

Gankra Oct 6, 2014

Gankra commented Oct 6, 2014

steveklabnik Oct 6, 2014

Gankra Oct 6, 2014

steveklabnik commented Oct 6, 2014

Gankra commented Oct 6, 2014

steveklabnik commented Oct 6, 2014

Gankra commented Oct 6, 2014

huonw Oct 6, 2014

Gankra Oct 6, 2014

Gankra commented Oct 6, 2014

Gankra commented Oct 6, 2014

Gankra commented Oct 6, 2014

aturon Oct 6, 2014

Gankra Oct 6, 2014

aturon Oct 6, 2014

Gankra Oct 6, 2014

aturon Oct 6, 2014

Gankra Oct 6, 2014

aturon Oct 6, 2014

aturon commented Oct 6, 2014

Gankra commented Oct 6, 2014

aturon commented Oct 6, 2014

Gankra commented Oct 6, 2014

aturon commented Oct 6, 2014

Gankra commented Oct 6, 2014

aturon commented Oct 6, 2014

steveklabnik commented Oct 6, 2014

huonw commented Oct 7, 2014

Gankra commented Oct 7, 2014

library-level docs for collections #17802

library-level docs for collections #17802

Conversation

Gankra commented Oct 5, 2014

Gankra commented Oct 5, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gankra commented Oct 6, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steveklabnik commented Oct 6, 2014

Gankra commented Oct 6, 2014

steveklabnik commented Oct 6, 2014

Gankra commented Oct 6, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gankra commented Oct 6, 2014

Gankra commented Oct 6, 2014

Gankra commented Oct 6, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aturon commented Oct 6, 2014

Gankra commented Oct 6, 2014

aturon commented Oct 6, 2014

Gankra commented Oct 6, 2014

aturon commented Oct 6, 2014

Gankra commented Oct 6, 2014

aturon commented Oct 6, 2014

steveklabnik commented Oct 6, 2014

huonw commented Oct 7, 2014

Gankra commented Oct 7, 2014