Deprecate Map.kt traverse & niche signatures, hygiene & bindAll #2971
Conversation
Kover Report
|
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/map.kt
Outdated
Show resolved
Hide resolved
nomisRev
changed the title
| mapOf("1" to 1, "2" to 2) | ||
| .align(mapOf("1" to 1, "2" to 2, "3" to 3)) { (_, a) -> | ||
| "$a" | ||
| } shouldBe mapOf("1" to "Ior.Both(1, 1)", "2" to Ior.Both(2, 2), "3" to Ior.Right(3)) |
There was a problem hiding this comment.
Shouldn't this code return
mapOf("1" to "Ior.Both(1, 1)", "2" to "Ior.Both(2, 2)", "3" to "Ior.Right(3)")
instead?
| buildMap(size) { | ||
| this@zip.forEach { (key, bb) -> | ||
| nullable { | ||
| put(key, map(key, bb, other[key].bind())) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Nit: bb represents a value of type A. Should we rename it to aa?
| buildMap(size) { | |
| this@zip.forEach { (key, bb) -> | |
| nullable { | |
| put(key, map(key, bb, other[key].bind())) | |
| } | |
| } | |
| } | |
| buildMap(size) { | |
| this@zip.forEach { (key, aa) -> | |
| nullable { | |
| put(key, map(key, aa, other[key].bind())) | |
| } | |
| } | |
| } |
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/map.kt
Outdated
Show resolved
Hide resolved
|
|
||
| @Deprecated( | ||
| "Traverse for Either is being deprecated in favor of Either DSL + Map.mapValues.\n$NicheAPI", | ||
| ReplaceWith( | ||
| "let<Map<K, A>, Either<E, Map<K, B>>> { m -> either<E, Map<K, B>> { m.mapValues<K, A, B> { (_, a) -> f(a).bind<B>() } } }", | ||
| "arrow.core.raise.either" | ||
| ) | ||
| ) | ||
| @OptIn(ExperimentalTypeInference::class) | ||
| @OverloadResolutionByLambdaReturnType | ||
| public inline fun <K, E, A, B> Map<K, A>.traverse(f: (A) -> Either<E, B>): Either<E, Map<K, B>> { | ||
| val acc = mutableMapOf<K, B>() | ||
| forEach { (k, v) -> | ||
| when (val res = f(v)) { | ||
| is Right -> acc[k] = res.value | ||
| is Left -> return@traverse res | ||
| } | ||
| } | ||
| return acc.right() | ||
| } | ||
| public inline fun <K, E, A, B> Map<K, A>.traverse(f: (A) -> Either<E, B>): Either<E, Map<K, B>> = |
There was a problem hiding this comment.
I've been replacing the use of traverse (i.e. here and it doesn't know how to infer the type of the f function 😞 :
(0..20_000).associateWith { it }.let<Map<Int, Int>, Either<Nothing, Map<Int, Int>>> { m ->
either {
m.mapValues<Int, Int, Int> { (_: Int, a: Int) ->
acc.add(a)
Either.Right<Any>(a).bind<Int>()
}
}
}And also suffer the issues if the use of f is using the implicit parameter it. It does no know how to infer types.
There was a problem hiding this comment.
Yes, I've seen the issue with Either.Right<Any> in multiple places (in our tests)😞 I think it's due to covariance and Nothing in the data class constructor. I think this is why Kotlin Std always requires specifying the arguments, and Nothing is never used anywhere.
If this constructor was operator fun fun <A, B> Either.Right.Companion.invoke(b: B): Either<A, B> = ... I think it would not have been a problem, but it comes with some other trade-offs like not being able to call ::Right.
That being said, I don't think a lot of people are using this pattern in real code, since traverse with only Right is map without Either.. In case that the compiler can know E and A I don't think this ReplaceWith results in any problems. So maybe we can leave this as best effort?
There was a problem hiding this comment.
Sure thing. I think we can leave as it is and be reactive in the case some raises an issue.
No description provided.