Skip to content

🎉 Add Either class, in Kotlin#51

Merged
luoser merged 6 commits intomasterfrom
either
Jan 27, 2017
Merged

🎉 Add Either class, in Kotlin#51
luoser merged 6 commits intomasterfrom
either

Conversation

@luoser
Copy link
Contributor

@luoser luoser commented Jan 26, 2017

heck yes!

Say hello to our first Kotlin class. 🤸‍♂️

what

I ran into this 🤔 💭 moment when setting up our code for native comment support on project updates: since we have comments for either projects or updates, and Swift in fact handles this with its own Either type, 💡, add one to android. Maybe even add it in Kotlin.

Things to note:

  • companion object allows for us to access left and right as static methods, while keeping our private constructor
    • this ensures that Either objects cannot be created with both values, i.e.
    new Either<Int, String>(1, "don't do this!")
  • != null checks are repeated rather than the defined isLeft and isRight functions to satisfy compiler optional warnings ¯_(ツ)_/¯
  • I stopped after adding mapLeft due to time, but coming soon will be flatten(), flatMap(), etc. when we have more time to figure out how to define these functions elegantly without enums

s/o to @mbrandonw for helping me explore this, and @christopherwright for the continued moral support

@luoser luoser changed the title Add Either class, in Kotlin 🎉 Add Either class, in Kotlin Jan 26, 2017
@luoser luoser requested a review from stephencelis January 26, 2017 20:44
@@ -0,0 +1,63 @@
package com.kickstarter.libs

class Either<out A, out B> private constructor (val left: A?, val right: B?) {
Copy link
Contributor

@stephencelis stephencelis Jan 26, 2017

Choose a reason for hiding this comment

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

What do you think of using sealed classes for this?

sealed class Either<out L, out R> {
  class Left<out L, out R>(val left: L) : Either<L, R>() {
    ...
  }
  class Right<out L, out R>(val right: R) : Either<L, R>() {
    ...
  }
}

Either.Left("Error!")
Either.Right(1)

when(either) {
  is Left -> either.left
  is Right -> either.right
}

Seems to be a common pattern for tagged unions in Kotlin, and prevents even the private ability of instantiating an invalid type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Contributor

@mbrandonw mbrandonw left a comment

Choose a reason for hiding this comment

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

amazing! the syntax is a bit funky, but i'll get used to it.

just a few questions...

@@ -0,0 +1,61 @@
package com.kickstarter.libs

sealed class Either<out A, out B> {
Copy link
Contributor

Choose a reason for hiding this comment

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

should everything be marked as public?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for posterity public is the default modifier in Kotlin so we get redundant warnings


@Test fun testEither_Right() {
val intOrString = Either.Right<Int, String>("hello")
assertEquals("hello", intOrString.right)
Copy link
Contributor

Choose a reason for hiding this comment

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

are the parens optional? should it not be intOrString.right()?

Copy link
Contributor

@christopherwright christopherwright left a comment

Choose a reason for hiding this comment

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

Looks great!

@luoser luoser merged commit 68211c2 into master Jan 27, 2017
@luoser luoser deleted the either branch January 27, 2017 17:53
@pakoito
Copy link

pakoito commented Jan 27, 2017

I have my reservations about the way Either is implemented. The use of null types, isLeft() and isRight() allows for a scape hatch from static analysis which should be enforced instead. The encoding to enforce should is similar to the Java version seen here https://github.com/pakoito/RxSealedUnions

EDIT: okay, I'm late to the discussion and it'd be great if you gave that project's README a look and gave me an opinion.

@stephencelis
Copy link
Contributor

stephencelis commented Jan 27, 2017

@pakoito Not sure I follow. Could you clarify? This is just a starting point and the API is based on our Swift implementation. The only nullables here come from the left() and right() fold operations, which we don't typically use outside of tests. In general we use either(ifLeft, ifRight) for folding.

EDIT: We also don't use isLeft() or isRight() in practice:

We could very well get rid of these interfaces.

@pakoito
Copy link

pakoito commented Jan 27, 2017

Fair enough them, but I'd remove all of them.

@stephencelis
Copy link
Contributor

For what it's worth, our implementations are all based on Haskell's Data.Either, which also implement these functions. The types should be statically sound, but if we're missing anything, let us know!

@pakoito
Copy link

pakoito commented Jan 27, 2017

map, mapRight, isLeft and isRight can all be expressed in terms of either(). I didn't know about Haskell's implementation, so probably I am being too anal about it.

class Left<out L, out R>(internal val left: L) : Either<L, R>()
class Right<out L, out R>(internal val right: R) : Either<L, R>()

fun <C> either(ifLeft: (A) -> C, ifRight: (B) -> C): C = when(this) {

Choose a reason for hiding this comment

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

Shouldn't this be inline? Also, this function may be called apply.

package com.kickstarter.libs

sealed class Either<out A, out B> {
class Left<out L, out R>(internal val left: L) : Either<L, R>()
Copy link

@d3xter d3xter Mar 19, 2017

Choose a reason for hiding this comment

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

I'd use Nothing as the unused type in Left and Right,
e.g. class Left< out L >(internal val left: L) : Either<L, Nothing>()

This allows to create an instance of Left with val left = Either.Left(15) without specifying the right type.

fun foo() = when {
1 == 1 -> Either.Left(1)
else -> Either.Right(Exception(""))
}

is correctly infered as Either<Int, Exception>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants