scala / scala-library-next

backwards-binary-compatible Scala standard library additions
Apache License 2.0
69 stars 17 forks source link

Idea: Add a `single` method to all collections' companion object #42

Open BalmungSan opened 3 years ago

BalmungSan commented 3 years ago

IMHO it is pretty common to wanting to create a collection of a single element. Whereas you can always use the apply that accepts a varargs, I personally would prefer to call a single method on the companion object of the collection I want.

My arguments are:

  1. I find such method to better show the intent.
  2. Such a method could be more efficient, especially because one would avoid the intermediate Seq that is allocated for the varargs.
  3. Iterator already has single.
NthPortal commented 3 years ago

I believe the purpose of single is to reduce the allocation/memory cost of an Iterator consisting of a single element. I'm not sure how we add an extension method that actually reduces allocation. I also don't think we want to add another method that companions are required to implement (that would have to wait until 3.1 anyway)

BalmungSan commented 3 years ago

I'm not sure how we add an extension method that actually reduces allocation.

Not sure what you mean with this.

I also don't think we want to add another method that companions are required to implement (that would have to wait until 3.1 anyway)

Maybe we could relax the constraints a little and just add it to most common ones? For example: List, Vector, Seq, ArraySeq, LazyList, Set and Map (this one accepting two parameters key & value instead of a tuple).

I believe all these can be added as extension methods for now, and when 3.1 comes they will be moved as normal methods on the companions. Also, AFAIK, for all it is straight forward to construct a single element instance.

NthPortal commented 3 years ago

Not sure what you mean with this.

sorry, I meant that I wasn't sure how to add such a method generically to something like IterableFactory with a sufficiently efficient implementation. the obvious generic implementation is (newBuilder += elem).result(), but it's not obvious to me that that's sufficiently better than the performance of from to be worth it. perhaps it is though, as some implementations of from require creating a builder anyway.

For example: [...] ArraySeq

i.ArraySeq does not actually get any efficiency benefit from this, as apply already wraps the varargs into an i.ArraySeq to make a Seq of them, so when apply forwards to from, there is no work left to do. without a dedicated implementation, i.ArraySeq would actually be less efficient.

BalmungSan commented 3 years ago

i.ArraySeq does not actually get any efficiency benefit from this, as apply already wraps the varargs into an i.ArraySeq to make a Seq of them

Oh pretty cool. Yeah, as I said I am talking more about beliefs and suppositions; for example for List it can be a :: Nil for Set and Map could be calling their 1 version directly, not sure if Vector could also benefit from this.

I guess the point of this issue is to see if there could be more collections that would be worth to have a single constructor.

julienrf commented 3 years ago

I’m on the fenced, I don’t think we get many benefits to write Seq.single(42) instead of Seq(42), in terms of readability. Also, personally, I prefer not having two ways to achieve the same thing. Performance-wise, I think List.apply has a special optimization that avoids allocating an intermediate Array. I would rather see this mechanism generalized to all the collections.

SethTisue commented 3 years ago

Performance-wise, I think List.apply has a special optimization that avoids allocating an intermediate Array. I would rather see this mechanism generalized to all the collections.

example of generalizing in that direction: https://github.com/scala/scala/pull/8790

neontorrent commented 3 years ago

I am all optimizing the apply method for all collections, but OP's proposal is a solution if the optimization cannot be implemented promptly as it is much easier.

morgen-peschke commented 3 years ago

It might be worth considering one instead of single, having the advantages of both brevity and familiarity for devs who use cats (which has equivalent methods like NonEmptyList.one)