Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CU-m3dyef Either, Option and Validated tap effect #2459

Merged
merged 3 commits into from
Jul 31, 2021
Merged

CU-m3dyef Either, Option and Validated tap effect #2459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
69ac86f
CU-m3dyef tap, tapLeft, tapInvalid
raulraja Jul 29, 2021
ad6a901
CU-m3dyef tap, tapLeft, tapInvalid (support for Option)
raulraja Jul 29, 2021
d46612b
remove unused import
raulraja Jul 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Either.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,12 +821,54 @@ public sealed class Either<out A, out B> {
* Example:
* ```
* Right(12).mapLeft { "flower" } // Result: Right(12)
* Left(12).mapLeft { "flower" } // Result: Left("flower)
* Left(12).mapLeft { "flower" } // Result: Left("flower")
* ```
*/
public inline fun <C> mapLeft(f: (A) -> C): Either<C, B> =
fold({ Left(f(it)) }, { Right(it) })

/**
* The given function is applied as a fire and forget effect
* if this is a `Left`.
* When applied the result is ignored and the original
* Either value is returned
*
* Example:
* ```
* Right(12).tapLeft { println("flower") } // Result: Right(12)
* Left(12).tapLeft { println("flower") } // Result: prints "flower" and returns: Left(12)
* ```
*/
public inline fun tapLeft(f: (A) -> Unit): Either<A, B> =
when (this) {
is Left -> {
f(this.value)
this
}
is Right -> this
}
Comment on lines +842 to +849
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also express this with a scoping function:

Suggested change
public inline fun tapLeft(f: (A) -> Unit): Either<A, B> =
when (this) {
is Left -> {
f(this.value)
this
}
is Right -> this
}
public inline fun tapLeft(f: (A) -> Unit): Either<A, B> = apply {
if(this is Left) {
f(value)
}
}

Is it easier to read? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this feels better it would apply to the other taps 🙂 Otherwise happy to be ignored 😅 #notablocker

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pablisco apply it's fine since I suppose the lambda has no cost, and its body is inlined since it's flagged as invoked once in its contract. I speculate it does not need to capture the state for the lambda. I'd be curious to see the resulting bytecode because the when statement I wrote has no allocations and most likely results in a table-switch indexed lookup.
In whatever case, It's not important to me. If you want to suggest some changes to the impl in the same branch, we can apply them before merging; I'm sure whatever users write in the lambda is gonna be more costly than any optimizations :)

Copy link
Contributor

@pablisco pablisco Jul 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raulraja Actually, now I think of it we could add the option to add effects for both sides of the bifunctor and make use of existing operations 🙂

  public inline fun onLeft(f: (A) -> Unit): Either<A, B> =
    onEither(left = f, right = {})

  public inline fun onRight(f: (B) -> Unit): Either<A, B> =
    onEither(left = {}, right = f)

  public inline fun onEither(left: (A) -> Unit, right: (B) -> Unit): Either<A, B> = 
    apply { fold(left, right) }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's probably allocating for each one of those lambdas? If that is the case, I'd rather have an alloc free impl that is less readable as we have no context as to how these functions will be used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is probably right. One option may be to use a decoy for the empty lambda if we don't have something like this already. Need to check the byte code :) 
As I said, unless someone else sees this useful, we can explore it later on

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think apply is confusing here since apply is typically for mutable instantiation logic, like builder syntax.
Which this definitely doesn't do.

I think the when code Raul has is the simplest 👍


/**
* The given function is applied as a fire and forget effect
* if this is a `Right`.
* When applied the result is ignored and the original
* Either value is returned
*
* Example:
* ```
* Right(12).tap { println("flower") } // Result: prints "flower" and returns: Right(12)
* Left(12).tap { println("flower") } // Result: Left(12)
* ```
*/
public inline fun tap(f: (B) -> Unit): Either<A, B> =
when (this) {
is Left -> this
is Right -> {
f(this.value)
this
}
}

/**
* Map over Left and Right of this Either
*/
Expand Down
42 changes: 42 additions & 0 deletions arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,48 @@ public sealed class Option<out A> {
Some.unit
) { b, c, d, _, _, _, _, _, _, _ -> map(b, c, d) }

/**
* The given function is applied as a fire and forget effect
* if this is a `None`.
* When applied the result is ignored and the original
* None value is returned
*
* Example:
* ```
* Some(12).tapNone { println("flower") } // Result: Some(12)
* none<Int>().tapNone { println("flower") } // Result: prints "flower" and returns: None
* ```
*/
public inline fun tapNone(f: () -> Unit): Option<A> =
when (this) {
is None -> {
f()
this
}
is Some -> this
}

/**
* The given function is applied as a fire and forget effect
* if this is a `some`.
* When applied the result is ignored and the original
* Some value is returned
*
* Example:
* ```
* Some(12).tap { println("flower") } // Result: prints "flower" and returns: Some(12)
* none<Int>().tap { println("flower") } // Result: None
* ```
*/
public inline fun tap(f: (A) -> Unit): Option<A> =
when (this) {
is None -> this
is Some -> {
f(this.value)
this
}
}

public inline fun <B, C, D, E> zip(
b: Option<B>,
c: Option<C>,
Expand Down
42 changes: 42 additions & 0 deletions arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Validated.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,48 @@ public sealed class Validated<out E, out A> {
public inline fun <EE> mapLeft(f: (E) -> EE): Validated<EE, A> =
bimap(f, ::identity)

/**
* The given function is applied as a fire and forget effect
* if this is `Invalid`.
* When applied the result is ignored and the original
* Validated value is returned
*
* Example:
* ```
* Valid(12).tapInvalid { println("flower") } // Result: Valid(12)
* Invalid(12).tapInvalid { println("flower") } // Result: prints "flower" and returns: Invalid(12)
* ```
*/
public inline fun tapInvalid(f: (E) -> Unit): Validated<E, A> =
when (this) {
is Invalid -> {
f(this.value)
this
}
is Valid -> this
}

/**
* The given function is applied as a fire and forget effect
* if this is `Valid`.
* When applied the result is ignored and the original
* Validated value is returned
*
* Example:
* ```
* Valid(12).tap { println("flower") } // Result: prints "flower" and returns: Valid(12)
* Invalid(12).tap { println("flower") } // Result: Invalid(12)
* ```
*/
public inline fun tap(f: (A) -> Unit): Validated<E, A> =
when (this) {
is Invalid -> this
is Valid -> {
f(this.value)
this
}
}

/**
* apply the given function to the value with the given B when
* valid, otherwise return the given B
Expand Down
33 changes: 30 additions & 3 deletions arrow-libs/core/arrow-core/src/commonTest/kotlin/arrow/core/EitherTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,17 @@ import arrow.core.test.generators.suspendFunThatThrows
import arrow.core.test.laws.FxLaws
import arrow.core.test.laws.MonoidLaws
import arrow.typeclasses.Monoid
import io.kotest.property.Arb
import io.kotest.property.checkAll
import io.kotest.matchers.shouldBe
import io.kotest.assertions.throwables.shouldThrow
import io.kotest.matchers.shouldBe
import io.kotest.property.Arb
import io.kotest.property.arbitrary.choice
import io.kotest.property.arbitrary.constant
import io.kotest.property.arbitrary.int
import io.kotest.property.arbitrary.long
import io.kotest.property.arbitrary.map
import io.kotest.property.arbitrary.negativeInts
import io.kotest.property.arbitrary.string
import io.kotest.property.checkAll

class EitherTest : UnitSpec() {

Expand Down Expand Up @@ -64,6 +65,32 @@ class EitherTest : UnitSpec() {
}
}

"tap applies effects returning the original value" {
checkAll(Arb.either(Arb.long(), Arb.int())) { either ->
var effect = 0
val res = either.tap { effect += 1 }
val expected = when (either) {
is Left -> 0
is Right -> 1
}
effect shouldBe expected
res shouldBe either
}
}

"tapLeft applies effects returning the original value" {
checkAll(Arb.either(Arb.long(), Arb.int())) { either ->
var effect = 0
val res = either.tapLeft { effect += 1 }
val expected = when (either) {
is Left -> 1
is Right -> 0
}
effect shouldBe expected
res shouldBe either
}
}

"fold should apply first op if Left and second op if Right" {
checkAll(Arb.intSmall(), Arb.intSmall()) { a, b ->
val right: Either<Int, Int> = Right(a)
Expand Down
31 changes: 29 additions & 2 deletions arrow-libs/core/arrow-core/src/commonTest/kotlin/arrow/core/OptionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ import arrow.core.test.laws.MonoidLaws
import arrow.typeclasses.Monoid
import io.kotest.matchers.shouldBe
import io.kotest.matchers.shouldNotBe
import io.kotest.property.checkAll
import io.kotest.property.Arb
import io.kotest.property.arbitrary.bool
import io.kotest.property.arbitrary.map
import io.kotest.property.arbitrary.int
import io.kotest.property.arbitrary.long
import io.kotest.property.arbitrary.map
import io.kotest.property.arbitrary.orNull
import io.kotest.property.arbitrary.string
import io.kotest.property.checkAll

class OptionTest : UnitSpec() {

Expand Down Expand Up @@ -75,6 +76,32 @@ class OptionTest : UnitSpec() {
} shouldBe None
}

"tap applies effects returning the original value" {
checkAll(Arb.option(Arb.long())) { option ->
var effect = 0
val res = option.tap { effect += 1 }
val expected = when (option) {
is Some -> 1
is None -> 0
}
effect shouldBe expected
res shouldBe option
}
}

"tapNone applies effects returning the original value" {
checkAll(Arb.option(Arb.long())) { option ->
var effect = 0
val res = option.tapNone { effect += 1 }
val expected = when (option) {
is Some -> 0
is None -> 1
}
effect shouldBe expected
res shouldBe option
}
}

"fromNullable should work for both null and non-null values of nullable types" {
checkAll { a: Int? ->
// This seems to be generating only non-null values, so it is complemented by the next test
Expand Down
26 changes: 26 additions & 0 deletions arrow-libs/core/arrow-core/src/commonTest/kotlin/arrow/core/ValidatedTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,32 @@ class ValidatedTest : UnitSpec() {
}
}

"tap applies effects returning the original value" {
checkAll(Arb.validated(Arb.long(), Arb.int())) { validated ->
var effect = 0
val res = validated.tap { effect += 1 }
val expected = when (validated) {
is Validated.Valid -> 1
is Validated.Invalid -> 0
}
effect shouldBe expected
res shouldBe validated
}
}

"tapInvalid applies effects returning the original value" {
checkAll(Arb.validated(Arb.long(), Arb.int())) { validated ->
var effect = 0
val res = validated.tapInvalid { effect += 1 }
val expected = when (validated) {
is Validated.Valid -> 0
is Validated.Invalid -> 1
}
effect shouldBe expected
res shouldBe validated
}
}

"zip is derived from flatMap" {
checkAll(
Arb.validated(Arb.long().orNull(), Arb.int().orNull()),
Expand Down