you could potentially optimise this for strict collections by doing

  def tapEach[U](f: A => U): C = knownSize match {
    case 0          => this
    case n if n < 0 => fromSpecific(new View.TapEach(this, f))
    case n if n > 0 =>
      foreach(f)
      this

it's not perfect (e.g. it misses List), but it might be better than nothing?

I think that this size optimization could result in surprising behaviour if the underlying collection is mutated but lazy. In that case, when the empty collection has elements inserted, and then the lazy collection forces, f will not be called. I do not know if there's actually any collections like this however...

I'm also not sure if we really need a View.TapEach, or can just do map(a => { f(a); a }) instead of fromSpecific(new View.TapEach(this, f))

You're right, I'll change this

can't return this?

Unfortunately, StrictOptimizedSeqOps[..., C] does not conform to C. I could play around with introducing a self-type to it, self: C => and see if there are any issues.

Ok, It seems that this solution does solve it, as long as you also enforce the self: C => type on all extending StrictOptimized{Seq,Set,Map,SortedMap, SortedSet}Ops. I'd like to get @julienrf 's opinion though if he forsees any issues with that constraint, maybe it's intended to be extendable by non-C things too

The exceptions used to be StringOps and ArrayOps. All regular collection types extend their C parameter. Can we make any further simplifications if we always enforce this? I don't think it's worth making this change specifically for tapEach. There are plenty of other methods that return C. In most cases you abstract over consuming a collection where this is not an issue. If you also need to produce a more specific collection type you need to use the proper abstraction anyway (unless we can change it in a more generic way).

@szeiger I can't think of any other improvements or simplifications that could be made if it had { self: C => unfortunately.

I think it would be simpler to return coll here (and keep using C as the return type).

you can make the LazyList independently lazy by doing LazyList.tabulate(5)(_ + 1)

The exceptions used to be StringOps and ArrayOps. All regular collection types extend their C parameter. Can we make any further simplifications if we always enforce this? I don't think it's worth making this change specifically for tapEach. There are plenty of other methods that return C. In most cases you abstract over consuming a collection where this is not an issue. If you also need to produce a more specific collection type you need to use the proper abstraction anyway (unless we can change it in a more generic way).

Could I get others' opinions on whether or not this should be implemented as a call to map instead of fromSpecific(new View.Map)? I think that the former simplifies/reduces the work for those implementing collections which need to override map, but I am curious to hear others' thoughts.

I don't think the decision should hold up the PR - it's an easy change to make in its own PR if needed.

I think using map is better, indeed.

Unfortunately, calling map { a => f(a); a } will only return a CC[A], rather than the more specific C.

That is interesting and unfortunate. I'm torn on whether or not it would still be better to use map and cast the result. I'm leaning towards not.

I asked @szeiger to look at this and explain it to me.

Josh's point is that we can't typecast our way out of this. CC and C[A] can be different collection types entirely:

scala 2.13.0-M5> (Map(1 -> "foo"): Iterable[(Int, String)]).map(x => x)
res5: Iterable[(Int, String)] = List((1,foo))

scala 2.13.0-M5> (collection.immutable.BitSet(0, 5, 9): Iterable[Int]).map(_ + 1)
res0: Iterable[Int] = Set(1, 6, 10)

(in an earlier edit, I'd included a String example where I "upcast" it to Iterable[Char], but that's not really the same, because it's an implicit conversion, not an upcast)

@SethTisue Wow! Very sneaky! Some downsides (which may have workarounds):

• filter can (as far as the compiler is concerned) change the length of the Iterable. So the consequences of this are that, perhaps non-strict Seq's like SeqView and LazyList will have to then go through each element in order to fulfill a call to apply(100), whereas before the filter call, they may have been able to independently evaluate/access element 100 immediately.
• The above point means that IndexedSeqView#filter cannot return an IndexedSeqView

To add on one more point, filter also destroys independent laziness of elements (currently only a thing for LazyList, but could potentially be a thing for some lazy IndexedSeq in the future).

@joshlemer okay, sounds like it’s better as-is then

@NthPortal I don’t follow?

@SethTisue

scala> def list = LazyList.tabulate(10)(i => i)
list: scala.collection.immutable.LazyList[Int]

scala> def f(i: Int) = println(i)
f: (i: Int)Unit

scala> list.map(i => {f(i); i})(5)
5
res0: Int = 5

scala> list.filter(i => {f(i); true})(5)
0
1
2
3
4
5
res1: Int = 5

I retract all complaints, and I'm happy with fromSpecific(new View.Map(...))