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.

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

add methods to Left and Right for upcasting the other type parameter #7011

Merged
merged 1 commit into from
Aug 8, 2018

Conversation

jozic
Copy link
Contributor

@jozic jozic commented Aug 7, 2018

This is another implementation of #6328 and #6329
based on suggestion by @lrytz

motivation: Left and Right constructors are typed as Left and Right respectively, but sometimes we want something like Option.empty which returns None but typed as Option.

The test represents a code that would not compile if plain Left/Right constructors are used

@SethTisue @lrytz please check it again, thanks!

EDIT: updated in #7080 to name the methods withLeft and withRight

@SethTisue
Copy link
Member

anyone have a better idea for the method name?

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @jozic!

@NthPortal
Copy link
Contributor

Is this worth having for None as well?

@jozic
Copy link
Contributor Author

jozic commented Aug 8, 2018

@NthPortal
there's Option.empty[A] for None and Option.apply[A] is sort of same for Some

@NthPortal
Copy link
Contributor

@jozic good point

@dwijnand
Copy link
Member

dwijnand commented Aug 8, 2018

If anyone has a better idea for the method name, please send a PR. 😄

@dwijnand dwijnand merged commit decbe79 into scala:2.13.x Aug 8, 2018
@jozic jozic deleted the either3 branch August 8, 2018 10:36
@jozic
Copy link
Contributor Author

jozic commented Aug 8, 2018

thanks @dwijnand

@SethTisue
Copy link
Member

gah I hate the name up so I guess I'd better try and think of something

pleased to have this landing, regardless

@NthPortal
Copy link
Contributor

I guess coming up with that will be Left.up to you Seth 😉

@dwijnand
Copy link
Member

dwijnand commented Aug 9, 2018

upcast is another option.

@lrytz
Copy link
Member

lrytz commented Aug 9, 2018

maybe widen

@joroKr21
Copy link
Member

joroKr21 commented Aug 9, 2018

widen would clash with a method on Functor in cats/scalaz which goes F[A] => F[B] for A <: B.

@smarter
Copy link
Member

smarter commented Aug 9, 2018

I would call it or, Left("empty").or[Int].

@dwijnand
Copy link
Member

dwijnand commented Aug 9, 2018

That would work really well if, given Either is right-biased, it had a companion object apply:

Either("foo").or[Int]

@NthPortal
Copy link
Contributor

another possibility, based on @smarter's suggestion, might be Left#orRight[R] and Right#orLeft[L]

@jozic
Copy link
Contributor Author

jozic commented Aug 10, 2018

how about withRight[Int] (withLeft[Int])
Left("foo").withRight[Int] (though it look much longer than Left("foo").up[Int] :))

the issue with or or orRight(orLeft) is that "or" implies some possibility of it to be Right where in fact it's a Left

SethTisue added a commit to SethTisue/scala that referenced this pull request Aug 15, 2018
@SethTisue
Copy link
Member

since M5 is imminent, I'm going to make an executive decision here and (for M5, anyway) go with @jozic's suggestion of withRight/withLeft: #7080

justification: if we expected this to be very commonly used, up would be nice, but otherwise, explicitness and readability win

as for withLeft versus orLeft, it's a minor difference, but I find @jozic's point convincing.

@SethTisue SethTisue changed the title add Left#up and Right#up to upcast to Either add methods to Left and Right for upcasting the other type parameter Aug 15, 2018
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Aug 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
8 participants