Add traverseWithStateM to Traverse

[andyscott]

Add traverseWithStateM and traverseWithStateMA to Traverse.

This allows you to carry a value through a computation without having to deal with the state monad directly. I have been using this perform multiple stateful computations at once, such as carrying the index value while also marking duplicates. The test I added demonstrates marking for duplicates.

For method naming, I chose traverseWithStateM because the "traverse" prefix seems most important. My only minor concern is that the "state" suffix implies the state monad being exposed.

[kailuowang]

What do you think about adding a version that returns the state as well? like G[F[(S, B)]]

[codecov-io]

Codecov Report

Merging #1767 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1767      +/-   ##
==========================================
+ Coverage   95.26%   95.26%   +<.01%     
==========================================
  Files         265      265              
  Lines        4283     4286       +3     
  Branches       97       94       -3     
==========================================
+ Hits         4080     4083       +3     
  Misses        203      203

Impacted Files	Coverage Δ
core/src/main/scala/cats/Traverse.scala	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16ea2ed...431097d. Read the comment docs.

[andyscott]

You can mostly achieve that with the current function just by pairing your result with the input state:

scala> val fa = List(1, 2, 3, 4, 2, 4, 6)
fa: List[Int] = List(1, 2, 3, 4, 2, 4, 6)

scala> fa.traverseWithStateM(Set.empty[Int])(
    (a, s) => if (s.contains(a)) Eval.now("dupe" -> s) else Eval.later(a.toString -> s))(
    (a, s, _) => s + a)
res0: Eval[List[(String, Set[Int])]] = ...

scala> res0.value                                                                  
res1: List[(String, Set[Int])] = List((1,Set()), (2,Set(1)), (3,Set(1, 2)),
  (4,Set(1, 2, 3)), (dupe,Set(1, 2, 3, 4)), (dupe,Set(1, 2, 3, 4)), (6,Set(1, 2, 3, 4)))                                                                 

The only disadvantage is that if you want to rely on the output of f for computing the next state, you have to destructure/access a tuple. Above I just throw away the value in (a, s, _) => s + a.

[kailuowang]

@andyscott right. thanks for coming up with an example.

[andyscott]

Sure thing. While the incremental state can be retained with the current function, the final output state is inaccessible. I don't know if it's worth it, but I could add a function that returns (S, G[F[B]]) instead of G[F[B]] by calling run instead of runA.

In this case, the above traverseWithStateM is adjusted to return (S, G[F[B]]) and traverseWithStateMA is added to return G[F[B]]. Or some other/better naming (because naming is hard!).

[kailuowang]

I am leaning towards adding the run version, it seems useful to me and we won't be able to add it when we lock binary compat.

[kailuowang]

Thanks so much! Let me see if I can get one more maintainer to review it in time for 1.0.0-MF

[johnynek]

I'm a bit negative on this method. What does it add vs the user calling this themselves? Can we show important examples where this method can be optimized?

[andyscott]

This method isn't really for me!

It mainly struck me as a bit cleaner to use and easier to work with for newer functional programmers. In this particular case, it removes a mental dependency on the state monad for the end user.

// note: haven't actually compiled this
// ... and it may not be the best example...

case class Record(id: Long, /* ... */)
case class S(index: Int, ids: Set[Long])

def doWork(in: Record): F[Result] = // ...

val ins: List[Record] = // ...

// with helper method
val outs1: F[List[Result]] =
  ins.statefulTraverse(S(0, Set.empty))(
    (rec, s) =>
      if (s.ids.contains(rec.id)) F.pure(DuplicateResult)
      else doWork(rec, s.index) )(
    (rec, s, _) => s.copy(
      index = s.index + 1,
      ids   = s.ids + rec.id) )

// without helper method
val outs2: F[List[Result]] =
  ins.traverse(rec => StateT[F, S, String](s => {
    val fb =
      if (s.ids.contains(rec.id)) F.pure(DuplicateResult)
      else doWork(rec, s.index))
    fb.map(_ => s.copy(
      index = s.index + 1,
      ids   = s.ids + rec.id) -> b)
  }).runA(S(0, Set.empty))

Some of the recent index related additions to Traverse could be implemented using this method.

[iravid]

Sorry to butt in, but wouldn't this be formulated more clearly with foldM?

[iravid]

E.g.

ins.foldLeftM(S(0, Set.empty)) { (s, rec) =>
  val fb =
    if (s.ids.contains(rec.id)) F.pure(DuplicateResult)
    else doWork(rec, s.index))
  fb.map(_ => s.copy(
    index = s.index + 1,
    ids   = s.ids + rec.id) -> b)
}

(sorry, was on mobile yesterday)

[andyscott]

The posted code example with foldLeftM doesn't work, as you need to fold into some single type B in monad G. You've got the state type as input but the computation type as output, i.e. S in, but G[Work] out when monadic folding mandates G[S] out.

With foldLeftM, I think the best you could do is mash up the state and computation values into a single result type B-- for example B: (S, F[List[String]]). You'd have to handle some type gymnastics in the fold, and you'd wind up with a F[(S, F[List[String]]). I suppose you could .map(_._2).flatten this to get the final result.

[iravid]

That's true. Thanks for the correction.

[andyscott]

Sure thing. You prompted me to think about the problem a bit more, which is always a good thing 😄 👍.

[kailuowang]

@johnynek is @andyscott's argument above convincing enough for you? If not, I am going to unschedule it from 1.0-RC1, we can add it through extension in a separate module or project.

[andyscott]

I'm not too concerned about this anymore (I'm indifferent if we merge it or not).

…

On Sep 26, 2017 08:19, "Kai(luo) Wang" ***@***.***> wrote: @johnynek <https://github.com/johnynek> is @andyscott <https://github.com/andyscott>'s argument above <#1767 (comment)> convincing enough for you? If not, I am going to unschedule it from 1.0-RC1, we can add it through extension in a separate module or project. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1767 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAS8WwF0Mk5lnVkhwjODKl__DnccN86_ks5smRX5gaJpZM4Oc8fO> .