optics-dev / Monocly

Optics experimentation for Dotty
MIT License
32 stars 5 forks source link

Base Foldable on toIterator instead of foldMap #52

Closed julien-truffaut closed 2 years ago

julien-truffaut commented 3 years ago

What about nonEmptyFoldMap? Could we also use toIterator and rely on the OpticCan flag to guarantee it is not empty?

Can we make toIterator lazy on Traversal?

julien-truffaut commented 2 years ago

I think that getAll should return a List and getOneOrMore a NonEmptyList. What I was wondering is if nonEmptyFoldMap in NonEmptyFoldImpl should be implemented in terms of toIterator, e.g.

protected[impl] def nonEmptyFoldMap[M: Semigroup](f: A => M)(s: S): M = {
  val it = toIterator(s)
  var state: M = f(it.next()) // could throw if S is empty but it shouldnt happen since we are in NonEmptyFoldImpl
  while (it.hasNext) {
     state = Semigroup[M].combine(state, f(it.next()))
  }
  state
}
julien-truffaut commented 2 years ago

I think Iterator is the right interface to generalise over lazy and non-lazy datastructure. Other option would be IterableOnce which is essentially Iterator plus an extra method to fast compute the length if possible. https://www.scala-lang.org/api/2.13.6/scala/collection/IterableOnce.html

The benefit of IterableOnce is that all collections extends it so that.getMany could accept a List or a Vector instead of asking the users to produce an Iterator.

kenbot commented 2 years ago

Oops, I just merged the other PR, there are conflicts you'll need to resolve