Add a guide on generic programming with collections #1088
Conversation
_data/overviews.yml
Outdated
| icon: building | ||
| url: "core/custom-collections.html" | ||
| by: Martin Odersky, Lex Spoon and Julien Richard-Foy | ||
| description: "In this document you will learn how the collections framework helps you define your own collections in a few lines of code, while reusing the overwhelming part of collection functionality from the framework." | ||
| - title: Generic Programming With Collections (Scala 2.13) |
There was a problem hiding this comment.
I think we should find a different title. The term "generic programming" in the context of Scala is claimed by shapeless. Reusing / overloading it can cause confusion.
Something like "Writing generic, external operations that integrate with all collection types"
There was a problem hiding this comment.
I agree that "generic programming" is probably not the best term here. Something a bit pithier and closer to the original would be "Programming With Collections Generically".
There was a problem hiding this comment.
Thanks, I’ve changed the title for “Programming with collections generically”
There was a problem hiding this comment.
still not a good title IMO, see my other comment on that
| to support `String` and `Array` values we have to use `IsSeqLike` instead. `IsSeqLike` is similar to | ||
| `IsIterableLike` but provides a conversion to `SeqOps[A, Iterable, C]` (for some types `A` and `C`). | ||
|
|
||
| Note that in the case `Seq` as well as `Map`, using `IsSeqLike` and `IsMapLike`, respectively, is also |
There was a problem hiding this comment.
The biggning beginning is a bit hard to read here
There was a problem hiding this comment.
I rephrased this as follows:
Using
IsSeqLikeis also required to make your operation work onSeqViewvalues, becauseSeqView
does not extendSeq. Similarly, there is anIsMapLiketype that makes operations work with
bothMapandMapViewvalues.
|
It might be worth comparing with this ancient blog post complaining about the difficulty of extending collections. |
|
@milessabin I think |
julienrf
force-pushed
the
collections-extension-methods
branch
from
8ab37ac
to
7d1ad95
Compare
| **Julien Richard-Foy** | ||
|
|
||
| This guide shows how to write operations that can be applied to any collection type and return the same | ||
| collection type, and how to write operations that can be parameterized by the type of collection to build. |
There was a problem hiding this comment.
"how to write operations that can be parameterized by the type of collection to build" isn't clear IMO, and doesn't get explained until quite a bit later. if a better wording can't be found, perhaps you could include one brief example of each of the two kinds of operations covered, maybe that's the best way to get the reader to "aha, I see".
| a `String` we want to get back another `String`, and so on. | ||
|
|
||
| To produce a collection whose type depend on a source collection, we have to use | ||
| `scala.collection.BuildFrom` (formerly `CanBuildFrom`) instead of `Factory`. |
There was a problem hiding this comment.
as always in this collections work, anytime BuildFrom appears, the snarky or skeptical reader will yell (silently, or perhaps on Twitter) "I thought you got rid of CanBuildFrom!
idk, but it might be worth expanding the parenthetical to a separate sentence like: "BuildFrom resembles the old collections API's CanBuildFrom, but appears much more rarely." idk.
There was a problem hiding this comment.
it's a bit of a tangent, but I wonder if the naming of Factory and BuildFrom could be more closely aligned with each other, since they are so closely related to each other. maybe it's too late.
|
|
||
| ## Summary | ||
|
|
||
| - To consume any collection, take an `IterableOnce` (or something more specific) as parameter, |
There was a problem hiding this comment.
It could be Seq, Set, Map, etc. as well, that’s why I didn’t want to restrict to just IterableOnce and Iterable.
|
@julienrf interested in returning to this...? |
|
I’m sorry, I forgot to address your feedback. Sure, I will fix that ASAP. |
julienrf
force-pushed
the
collections-extension-methods
branch
2 times, most recently
from
82105d2
to
72e0a40
Compare
|
I updated the PR with most of the feedback addressed. I think there are still remarks that I didn’t really address because I couldn’t find a good solution. I’m happy to take suggestions… I’ve also re-tested all code examples with the latest Scala-2.13.x version. |
julienrf
force-pushed
the
collections-extension-methods
branch
2 times, most recently
from
eefb77f
to
5f35553
Compare
julienrf
force-pushed
the
collections-extension-methods
branch
from
5f35553
to
a384367
Compare
|
I don’t understand why the page is not visible: https://docs.scala-lang.org/overviews/core/custom-collection-operations.html |
|
when I generate the site locally (after an hour of wrestling with #1150 and #1286), the page appears. @jvican are you the keeper of the site build? I don't see anything obviously amiss at https://platform-ci.scala-lang.org/scala/docs.scala-lang ? |
|
I don't know what's going on, but it doesn't look related to the build. Are we sure the doc here has been added to all the required indices to show up in the overview? |
|
@jvican it shows up when I build the site locally. that's why I suspect some problem in the deploy pipeline |
|
i doubt it, the site is rebuilt from scratch, no incremental rebuild is enabled... i'm hitting the bed now, but perhaps cloning + pushing a new commit would make the guide show up? if this PR or other PRs merged after this one weren't properly rebased, maybe that could have caused problems? |
|
I pushed a dummy empty commit: c3f1423 the resulting run was https://platform-ci.scala-lang.org/scala/docs.scala-lang/1131 again, everything looks normal in the Drone log. I still think this has to be a problem in the deploy pipeline |
|
@jvican ping. this isn't super urgent, but also, we should try to get it straightened out in time for RC1, which is coming up on us fairly fast |
|
@SethTisue Our great sysadmin @fsalvi fixed the problem! |
|
sweet, thanks Fabien! |
Note: this PR depends on scala/scala#6674. If that one does not get merged I will rework it.