List's traverse method was applying effects in reverse.

[non]

This commit fixes this issue (which had to do with passing
arguments to G.map2 in the wrong order) and adds a regression
test to be sure we don't accidentally mess this up in the
future.

Fixes #140

Review by @refried et al.

[mpilquist]

👍

[julien-truffaut]

could we make the same test on List[Option[Int]]?

[non]

yeah it should be possible.

[ceedubs]

I think this implementation of traverse is problematic when side effects are in play and should be executed in order. This is a problem even if the side effects are properly housed within something like IO or Task.

Example with scalaz's Task:

scala> traverse(List(1, 2, 3))(i => Task{println(i); i})
res2: scalaz.concurrent.Task[List[Int]] = scalaz.concurrent.Task@3b4c8039

scala> res2.run
3
2
1
res3: List[Int] = List(1, 2, 3)

I don't think you can rely on running side effects in a map call in the general case.

[non]

I don't think this is about side-effects, but just about the order in which the monad instances (which constrain effects) are composed, right?

EDIT: If you add a println to the alloc method in the test you'll see that things are printed in the "right" order.

[wedens]

scalaz/scalaz#886 discussion in scalaz

[non]

The reason Scalaz hits this problem is because they need to reverse the list to do a foldLeft. Since we are building up a result in ListBuffer we can build a result directly with foldLeft so we don't have the same issue.

That said, I agree that side-effecting during the fold is a bad idea -- the reason to avoid reversing is to minimize garbage IMO.

[ceedubs]

@wedens I would consider my example different than the one you have linked because I'm not using traverse directly for side effects. I'm traversing into Task (so basically IO), which as far as I know is the standard way to sequence a list of side-effecting evaluations.

@non sorry I'm not sure I completely understand your response. Are you agreeing that this is an issue, or does it not seem like a problem to you?

I have no qualms about local side effects for performance reasons. I just am not sure that you can do side effects (without using IO or something like it) in an arbitrary Applicative's map2 method in a reliable way.