[PROPOSAL] Option API deprecation, and preparation for 2.x.x #2913

franciscodr · 2023-02-02T16:58:18Z

This PR proposes a smaller API surface for Option without compromising its functionality or feature set.
The goal of these changes is to offer APIs that are more powerful and more idiomatic to use in Kotlin. Following the signatures and naming of some other well-known APIs from Kotlin Std, or KotlinX Coroutines

Option (API Size 70 -> 21)

Foldable / Traverse (API Size 18 -> 1)

Keep

fold

Remove

crosswalk: replace with map { f(it) }
crosswalkMap: replace with fold( { emptyMap() }, { a -> f(a).mapValues { Some(it.value) } })
crosswalkNull: replace with map { value -> f(value)?.let { Some(it) } }.orNull()
foldLeft: replace with fold
foldMap: replace with fold
reduceOrNull: replace with map { value -> operation(initial(value), value) }.orNull()
reduceRightEvalOrNull: replace with fold({ Eval.now(null) }, { value -> operation(value, Eval.now(initial(value))) })
separateEither: replace with fold({ None to None }) { either -> either.fold<Pair<Option<A>, Option<B>>>({ Some(it) to None }, { None to Some(it) }) }
separateValidated: replace with fold({ None to None }) { validated -> validated.fold<Pair<Option<A>, Option<B>>>({ Some(it) to None }, { None to Some(it) }) }
unite: replace with map { iterable -> iterable.fold(MA) }
uniteEither: replace with flatMap { either -> either.fold({ None }, { Some(it) }) }
uniteValidated: replace with flatMap { validated -> validated.fold({ None }, ::Some) }
All the traverse methods can be replaced by a simple fold, or even simpler by orNull, getOrElse in many cases.
- sequence (Iterable)
- sequenceEither
- sequence (Either)
- sequenceValidated
- sequence (Validated)
- traverse (Iterable)
- traverse Either
- traverse (Either)
- traverseValidated
- traverse (Validated)

Functor / Applicative / Monad (API Size 21 -> 10)

New

onSome: evaluate a fire-and-forget action if the value is defined
onNone: evaluate a fire-and-forget action if the value is empty

Keep

map: transform the value if defined
filter: keep the value if it holds the predicate. Otherwise, return None
filterNot: keep the value if it doesn't hold the predicate. Otherwise, return None
flatMap: transform the value if defined with Option + flatten
flatten: flatten nested option values
unzip: convert an option value containing a pair into a pair with two option values
widen: conveniently change the generic parameters to a superclass type

Remove

all: replace with fold({ false }, predicate) or map(predicate).getOrElse{ false }
ensure: use instead DSL + ensure, flatMap + if-else
exists: replace with fold({ true }, predicate) or map(predicate).getOrElse{ true }
findOrNull: replace with getOrNull()?.takeIf(predicate)
pairLeft: replace with map { left to it }
pairRight: replace with map { it to right }
replicate: replace with map { List(n) { it } } // There's an alternative with Monoid
tap -> replace with onDefined to be more in line with Kotlin Std
tapNone -> replace with onEmpty to be more in line with Kotlin Std
unit: replace with Some(Unit)
void: replace with map {}

Replace

zip: the original zip implementation will be replaced with inline option + bind

Applicative/MonadError (API Size 5 -> 0)

Remove

handleError: replace with getOrElse { f(Unit) }
handleErrorWith: replace with orElse { f(Unit) }
redeem: replace with map(fb).orElse { Some(fe(Unit)) }
redeemWith: replace with flatMap(fb).orElse(fe)
rethrow: replace with flatMap { it.fold({ None }, { a -> Some(a) }) }

Align (API Size 7 -> 0)

Remove

align: prefer using a simple fold, or when expression
padZip: prefer using a simple fold, or when expression
salign: prefer using a simple fold, or when expression
unalign: prefer using a when expression

Utilities (API Size 19 -> 10)

New

getOrNull: extract the value or return null

Keep

compareTo: keep as Comparable operator
getOrElse: extract or provide a fallback value if empty
isEmpty: check if the value is not defined
isNotEmpty: check if the value is defined
orElse: return the option value if defined. Otherwise, return the provided fallback value
toEither: convert the option value as Either.Right if defined. Otherwise, return the provided fallback value as Either.Left
toList: convert the option value as a singleton list if defined. Otherwise, return an empty list
toMap: convert the option value containing a pair as a singleton map if defined. Otherwise, return an empty map
T?.toOption(): convert a nullable value into None if it's null. Otherwise, return Some containing the value

Remove

and: replace with flatMap { value }
Boolean.maybe: replace with if (this) { Some(f()) } else { None }
combine: will be covered by accumulating zip behavior in 2.x.x
combineAll: replace with fold(Monoid.option(MA))
filterIsInstance: replace with flatMap { when (it) { is B -> Some(it) else -> None } }
isDefined: duplicate - use isNotEmpty
mapNotNull: replace with flatMap { fromNullable(f(it)) }
nonEmpty: duplicate - use isNotEmpty
or: replace with orElse
orNull: replace with getOrNull

github-actions · 2023-02-02T17:11:37Z

Kover Report

File	Coverage [51.33%]
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Either.kt	57.04%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt	44.30%

Total Project Coverage	43.72%

nomisRev

Looks good to me so far. I think it was missing some imports in the ReplaceWith 🤔

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt

Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>

franciscodr · 2023-02-06T12:41:24Z

@nomisRev Do we want to deprecate the zip method and all its variants with the inline option DSL? So far, I kept the methods and only changed the implementation to use the DSL, but I saw you're deprecating the zip method for the Either type.

nomisRev · 2023-02-06T13:13:54Z

Do we want to deprecate the zip method and all its variants with the inline option DSL?

Yes, we agreed that fa.zip(fb) { a, b -> transform(a, b) } doesn't really offer any benefits over option { transform(fa.bind(), fb.bind()) }. The latter is only 4 characters longer than the former, and doesn't suffer from the arity-n problem.

It's an API that originally came from Apply, but offers duplication in the API as bind (or flatMap).

nomisRev · 2023-02-06T14:08:13Z

This decision can still be challenged of course. @myuwono would be great to get your review here when it's ready for review. He's one of the biggest users of Option in combination with Spring WebFlux, and can perhaps short-circuit some things if needed.

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt

serras

LGTM

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt

myuwono · 2023-02-06T20:12:21Z

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt

@@ -1029,6 +1255,14 @@ public inline fun <A> Option<A>.ensure(error: () -> Unit, predicate: (A) -> Bool
 /**
 * Returns an Option containing all elements that are instances of specified type parameter [B].
 */
+@Deprecated(
+  NicheAPI + "Prefer using option DSL or flatMap",


This aligns with stdlib for list.filterIsInstance. let's not deprecate this.

Is this an API that is commonly used in your projects @myuwono? It's an API I personally find kind-of strange on Option, even on List I think it's not commonly used 🤔

I am having a hard time thinking of a good use-case. I hope that Option<Any> is an extremely rare case 😁
Otherwise inheritance rules should apply? 🤔

Similarly to Lists, this is useful for a call that returns a sealed type @nomisRev. For instance if a database call returns Option<Document> where document is a sealed class with many subtypes, and we are interested in only one of them e.g. Option<Document.ExternalMetadata>.

Right, you need it for narrow'ing. So an Option alternative to as?.
Since this only makes sense for Option, lets keep it 👍

Thanks for the feedback, @myuwono! I have reverted the deprecation in this commit

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt

nomisRev

Couple of thoughts, and nits. We should do a pass for the contracts in a subsequent PR, I think we might have missed some.

I'm going to approve, we can keep discussion open with @myuwono and make adjustments later on ☺️

For anyone reading this, anything deprecated now can be challenged before 2.x.x release. Or can be re-introduced if needed. Please provide and share use-cases, and production usage ☺️

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt

nomisRev · 2023-02-07T13:03:17Z

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt

@@ -1029,6 +1255,14 @@ public inline fun <A> Option<A>.ensure(error: () -> Unit, predicate: (A) -> Bool
 /**
 * Returns an Option containing all elements that are instances of specified type parameter [B].
 */
+@Deprecated(
+  NicheAPI + "Prefer using option DSL or flatMap",


Is this an API that is commonly used in your projects @myuwono? It's an API I personally find kind-of strange on Option, even on List I think it's not commonly used 🤔

I am having a hard time thinking of a good use-case. I hope that Option<Any> is an extremely rare case 😁
Otherwise inheritance rules should apply? 🤔

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt

nomisRev · 2023-02-07T13:30:20Z

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt

+  @Deprecated(
+    NicheAPI + "Prefer Kotlin nullable syntax instead",
+    ReplaceWith("getOrNull()?.takeIf(predicate)")
+  )
  public inline fun findOrNull(predicate: (A) -> Boolean): A? {


Pretty crazy.. these APIs predate the introduction of takeIf into Kotlin Std. Arrow is getting old, happy it's getting a new lack of paint 😍

nomisRev

Temporarily blocking this PR pending the ongoing conversation with @myuwono.

nomisRev · 2023-02-09T10:23:05Z

arrow-libs/core/arrow-core/api/arrow-core.api

@@ -867,7 +867,6 @@ public final class arrow/core/NonFatalOrThrowKt {

 public final class arrow/core/None : arrow/core/Option {
 	public static final field INSTANCE Larrow/core/None;
-	public fun isEmpty ()Z


@franciscodr sorry for dragging this PR on so long 🙈
We need to revert the change we made to abstract fun isEmpty in order to not break binary compatibility. The improvements we made in isSome.

No need to apologize, @nomisRev. Thanks for your thorough review of this PR 😄

nomisRev

Thank you @franciscodr for this awesome PR 🙏 👏 🥳 And thanks @myuwono for the great feedback and brainstorming the new API 🎉

nomisRev reviewed Feb 3, 2023

View reviewed changes

Deprecate foldable methods

c675811

franciscodr force-pushed the fd-option-API branch from 9a4e423 to c675811 Compare February 3, 2023 19:20

franciscodr and others added 13 commits February 4, 2023 09:27

Deprecate functor/applicative/monad methods

268c995

Update API files

8858cec

Apply suggestions from code review

ea87f40

Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>

Add getOrNull method

fb8a3aa

Update API files

439773d

Restore isEmpty implementation

2d4cb40

Update API files

4165ee9

Deprecate applicative/monad error methods

8e8c092

Deprecate utility methods

78b4384

Merge remote-tracking branch 'origin/main' into fd-option-API

e9ce01b

Merge remote-tracking branch 'origin/main' into fd-option-API

37bbc40

Deprecate assign methods

356a273

Fix outdated knit files

7fc257e

franciscodr and others added 2 commits February 6, 2023 13:44

Merge branch 'main' into fd-option-API

3288486

Merge branch 'main' into fd-option-API

1d1ae2c

Using the inline option DSL

8198fce

nomisRev reviewed Feb 6, 2023

View reviewed changes

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt Show resolved Hide resolved

nomisRev reviewed Feb 6, 2023

View reviewed changes

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt Outdated Show resolved Hide resolved

franciscodr added 2 commits February 6, 2023 20:05

Change redeem implementation for Either and Option

b0f9ff1

Merge branch 'main' into fd-option-API

b05b8ea

franciscodr marked this pull request as ready for review February 6, 2023 19:06

franciscodr requested a review from a team February 6, 2023 19:07

serras approved these changes Feb 6, 2023

View reviewed changes

myuwono reviewed Feb 6, 2023

View reviewed changes

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt Outdated Show resolved Hide resolved

myuwono reviewed Feb 6, 2023

View reviewed changes

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt Show resolved Hide resolved

i-walker approved these changes Feb 7, 2023

View reviewed changes

franciscodr added 2 commits February 7, 2023 14:21

Fix wrong deprecation replacement

49a018b

Revert Nullable.toOption deprecation

58b51a4

nomisRev approved these changes Feb 7, 2023

View reviewed changes

franciscodr and others added 2 commits February 7, 2023 15:59

Add missing contracts

3936647

Update API files

241d601

nomisRev added the 1.2.0 Tickets belonging to 1.1.2 label Feb 7, 2023

nomisRev requested changes Feb 8, 2023

View reviewed changes

nomisRev and others added 7 commits February 8, 2023 11:38

Merge branch 'main' into fd-option-API

b4fad18

Revert filterIsInstance deprecation

d4f5bc4

Add isNone and isSome methods for Option

df95be0

Update API files

6599bf3

Trigger build

6408a19

Update API files

4a9c115

Convert isSome into an instance method

965395b

nomisRev mentioned this pull request Feb 9, 2023

Introduce isLeft & isRight predicate API for Either #2927

Merged

nomisRev reviewed Feb 9, 2023

View reviewed changes

franciscodr added 2 commits February 9, 2023 15:49

Restore abstract isEmpty function for Option

3abcb1e

Merge branch 'main' into fd-option-API

2911df7

nomisRev approved these changes Feb 9, 2023

View reviewed changes

Merge branch 'main' into fd-option-API

cf57557

nomisRev added this pull request to the merge queue Feb 9, 2023

Merged via the queue into main with commit 50c80d5 Feb 9, 2023

franciscodr deleted the fd-option-API branch February 9, 2023 14:55

franciscodr mentioned this pull request Feb 9, 2023

["Request"] Option API deprecations #2888

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PROPOSAL] Option API deprecation, and preparation for 2.x.x #2913

[PROPOSAL] Option API deprecation, and preparation for 2.x.x #2913

franciscodr commented Feb 2, 2023 •

edited

Loading

github-actions bot commented Feb 2, 2023 •

edited

Loading

nomisRev left a comment

franciscodr commented Feb 6, 2023

nomisRev commented Feb 6, 2023 •

edited

Loading

nomisRev commented Feb 6, 2023

serras left a comment

myuwono Feb 6, 2023

nomisRev Feb 7, 2023

myuwono Feb 7, 2023 •

edited

Loading

nomisRev Feb 8, 2023

franciscodr Feb 8, 2023

nomisRev left a comment

nomisRev Feb 7, 2023

nomisRev Feb 7, 2023

nomisRev left a comment

nomisRev Feb 9, 2023

franciscodr Feb 9, 2023

nomisRev left a comment

[PROPOSAL] Option API deprecation, and preparation for 2.x.x #2913

[PROPOSAL] Option API deprecation, and preparation for 2.x.x #2913

Conversation

franciscodr commented Feb 2, 2023 • edited Loading

Option (API Size 70 -> 21)

Foldable / Traverse (API Size 18 -> 1)

Keep

Remove

Functor / Applicative / Monad (API Size 21 -> 10)

New

Keep

Remove

Replace

Applicative/MonadError (API Size 5 -> 0)

Remove

Align (API Size 7 -> 0)

Remove

Utilities (API Size 19 -> 10)

New

Keep

Remove

github-actions bot commented Feb 2, 2023 • edited Loading

Kover Report

nomisRev left a comment

Choose a reason for hiding this comment

franciscodr commented Feb 6, 2023

nomisRev commented Feb 6, 2023 • edited Loading

nomisRev commented Feb 6, 2023

serras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

myuwono Feb 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nomisRev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nomisRev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nomisRev left a comment

Choose a reason for hiding this comment

franciscodr commented Feb 2, 2023 •

edited

Loading

github-actions bot commented Feb 2, 2023 •

edited

Loading

nomisRev commented Feb 6, 2023 •

edited

Loading

myuwono Feb 7, 2023 •

edited

Loading