Implement OptionT#foreachF

[armanbilge]

No description provided.

[satorg]

Looks good and reasonable to me, thanks.

Would it be too much trouble to add a sort of "stabilizing" test case to OptionTSutie please? I don't really think it is absolutely necessary here, but I believe it is always nice to have.

[satorg]

Just an idea: would you consider by chance to add a similar foreachF to EitherT for consistency with Either and Option (since Either has a regular foreach as well as Option)?

[armanbilge]

Thanks, yes I can add a test, sorry for being lazy :) what do you mean by "stabilizing"? I was thinking a prop testing its identity with fold.

Regarding EitherT, I can see the argument for consistency but IMHO foreach on Either seems like an anti-pattern that I personally would never use (since it silently loses/ignores the left). The whole point of Either is to preserve both channels, if someone really wants to do this I think you should just convert to Option first (this will make it very clear you are losing the error channel) and then do foreach. But just my opinion :)

[satorg]

what do you mean by "stabilizing"?

I just meant although the implementation seems straightforward, but it might be useful to "pin" it to make sure it won't be accidentally broken in the future. The Prop based test you mentioned would be just great.

Regarding EitherT, I can see the argument for consistency but IMHO foreach on Either seems like an anti-pattern that I personally would never use (since it silently loses/ignores the left). The whole point of Either is to preserve both channels, if someone really wants to do this I think you should just convert to Option first (this will make it very clear you are losing the error channel) and then do foreach. But just my opinion :)

Agree, that makes sense to me.

[johnynek]

side note, this foldMap where B =:= F[Unit]. But I think we can't implicitly resolve Monoid[F[A]] given F[_]: Applicative (since I don't think that resolves without additional imports).

[armanbilge]

Interesting note!

[johnynek]

B type parameter is unused. Can we remove it?

[armanbilge]

Oops, right. Thanks.

Suggested change

	def foreachF[B](f: A => F[Unit])(implicit F: Monad[F]): F[Unit] =
	def foreachF(f: A => F[Unit])(implicit F: Monad[F]): F[Unit] =