Add bind alias for Sequence operation in DSL

[nomisRev]

Adds bind as suggested by @pakoito in #2861.
Updates sequence deprecation to point towards DSL.

[github-actions]

Kover Report

File	Coverage [80.71%]
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Iterable.kt	84.74%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/Builders.kt	97.62%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/Raise.kt	72.73%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/RaiseAccumulate.kt	62.73%

Total Project Coverage	43.27%

[serras]

I'm not a fan of reusing the bind name for both Iterable and non-Iterable versions.

[nomisRev]

I'm not a fan of reusing the bind name for both Iterable and non-Iterable versions.

I named it bind simply because it didn't have a conflict (in signature) with the other bind 😅
If you like the API, but dislike the name we can also rename it. @pakoito originally proposed bindAll.

[serras]

I'm not a fan of reusing the bind name for both Iterable and non-Iterable versions.

I named it bind simply because it didn't have a conflict (in signature) with the other bind 😅 If you like the API, but dislike the name we can also rename it. @pakoito originally proposed bindAll.

I would prefer that name, indeed.

[nomisRev]

@serras one follow-up question, for RaiseAccumulate this should be overwritten and use accumulate strategy for bindAll, right?

[serras]

@serras one follow-up question, for RaiseAccumulate this should be overwritten and use accumulate strategy for bindAll, right?

That would be great! I thought that merely calling bind inside RaiseAccumulate would be enough to have this behavior, but if we need to overload bindAll there too, I would make the change too.

[nomisRev]

Calling bind within the lambda still short_circuits the "current op".

I.e.

listOf(1, 2).mapOrAccumulate {
  if(it == 1) {
     "fail-1".left().bind() // <-- short-circuits "current op"
     "fail-1".left().bind() // this is not accumulated
  } else {
    listOf("fail-2".left(), "fail-2".left()).bindAll() // both get accumulated
  }
} // Either.Left(NonEmptyList("fail-1", "fail-2", "fail-2"))

So bindAll needs to have a specialised implementation since map { it.bind() } would result in the 1 scenario. Or am I overlooking something?

[serras]

@nomisRev It makes sense to me, that due to the way types are resolved the bind() gets called on Raise<E> instead of RaiseAccumulate<E>. It still needed a bit of additional thinking, though, it doesn't seem entirely obvious to me. So good point that we're adding it to RaiseAccumulate, because that's the least surprising behavior.

[nomisRev]

@serras I am not sure I entirely follow your train of though.

It makes sense to me, that due to the way types are resolved the bind() gets called on Raise instead of RaiseAccumulate.

Isn't this irrelevant? There is no other way for bind to work? What would the semantics otherwise be? Since bind returns A it has to short-circuit, right? Otherwise it would have to return something like Value which you modelled in Accumulate DSL. & thus de-coupling the DSL completely from Raise.

It still needed a bit of additional thinking, though, it doesn't seem entirely obvious to me.

Not sure I understood this comment. Is it the short-circuit behavior inside of accumulate that requires additional thinking? Or the fact that semantics of bind remain the same and bindAll semantics change depending on being inside Raise or RaiseAccumulate?

[nomisRev]

Here are the two tests that show the behavior @serras

[franciscodr]

Just a few typos. Thanks, @nomisRev!

[pakoito]

the let is unnecessary here, right?

[pakoito]

option { this@sequence.bindAll() }

[nomisRev]

Yes, indeed it's so that the ReplaceWith is inline with the implementation body if people click through. Without let ReplaceWith horribly suffers from scope pollution 😞

let should incur any perf penalty due to @InlineOnly.

[pakoito]

I was wondering why I got a Mentioned notification :D Good to hear my suggestion made it <3

On the mini-arrow I made for TypeScript we have traverse for Either<E[], A[]>, traverseFailFast for Either<E, A[]> and traverseSeparate for a surprisingly useful Pair<E[], A[]>.

[nomisRev]

We have traverse separate but it’s called separateEither since we also have separateIor (and now deprecated separateValidated).

It’s indeed very useful, we’ve had it since before 1.0.0 though. Originally in Alternative typeclass, and then as extension on Iterable.

And ofc, your feedback and knowledge is always super valuable @pakoito ❤️ Miss you 😘