-
Notifications
You must be signed in to change notification settings - Fork 451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NonEmptyList: Add bindAll and more unit tests #2995
Conversation
@@ -84,6 +85,11 @@ public class ResultRaise(private val raise: Raise<Throwable>) : Raise<Throwable> | |||
@JvmName("bindAllResult") | |||
public fun <A> Iterable<Result<A>>.bindAll(): List<A> = | |||
map { it.bind() } | |||
|
|||
@RaiseDSL | |||
@JvmName("bindAllResult") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may have a problem with this name clashing in the Arrow 2.0 branch, where NonEmptyList
is defined as a value class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how it would differ when it being a value class
🤔 From Kotlin AST point-of-view it's a different type, and from JVM binary it has a different name.
@@ -249,6 +250,10 @@ public interface Raise<in Error> { | |||
@RaiseDSL | |||
public fun <A> Iterable<Either<Error, A>>.bindAll(): List<A> = | |||
map { it.bind() } | |||
|
|||
@RaiseDSL | |||
public fun <A> NonEmptyList<Either<Error, A>>.bindAll(): NonEmptyList<A> = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are including NonEmptyList
, should we also include Set
and other types of collections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I thought about adding NonEmptySet
too today
Kover Report
|
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/RaiseAccumulate.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great 🙌 Thank you @franciscodr.
This pull request adds the
bindAll
function toNonEmptyList
for bothRaise
andRaiseAccumulate
implementations. Additionally, it adds missing unit tests for the NonEmptyList API.