Skip to content

Add scala.Try as alias of scala.util.Try to scala package. #7549

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

Closed
wants to merge 1 commit into from

Conversation

He-Pin
Copy link
Contributor

@He-Pin He-Pin commented Dec 17, 2018

  • Add scala.Try type and value aliases (just like for Either), but not for Success/Failure.
  • Move scala.util.Success and scala.util.Failure into the scala.util.Try companion object
  • Add deprecated type/value aliases scala.util.Success = scala.util.Try.Success, same for Failure. The deprecation message should recommend using Try.Success/Try.Failure (instead of importing them).

@lrytz
Copy link
Member

lrytz commented Dec 17, 2018

Could you squash the two commits?

@He-Pin
Copy link
Contributor Author

He-Pin commented Dec 17, 2018

@lrytz Squashed.

@som-snytt
Copy link
Contributor

After minimal reflection, moving success/failure means I can't wildcard import because of apply pollution. That seems inconvenient, and a non-starter.

Green field, I'd call them Tried and Thrown, which do not impose a judgment on the result.

@SethTisue
Copy link
Member

After minimal reflection, moving success/failure means I can't wildcard import because of apply pollution. That seems inconvenient, and a non-starter

hmm, not sure if that bothers me, since style-wise Try.Success and Try.Failure seems superior to me anyway, there are multiple notions of "success" and "failure" in play around practically any nontrivial file of Scala code

@som-snytt
Copy link
Contributor

som-snytt commented Jan 10, 2019

I see there is no forum to obtain more opinions? no ticket at all?

Edit: I think aliasing Try is an incremental win, but I don't much like nested Success, Failure because it adds work instead of subtracting it.

I don't strongly disagree, except flu symptoms make me ornery: Try.Success is ugly. Scoping to scala.util suffices where I can import util._ without picking up apply, which is a cogent objection but maybe doesn't matter much because how often do you write explicit unprefixed apply(x)?

I would always import Try.{Success, Failure}. Otherwise, the recommendation should be (as with Option) just use map, recoverWith, etc.

The reason to pattern match is to avoid allocation?

expr match { case t @ Success(_) => t }

The other day, and maybe it was the cold medicine talking, I was thinking about success as successor, that is, any result is a good outcome, where the alternative is death and the end of the line of succession.

There's also a colorful quote from William James online. They suggest that "a person is a success or failure" is an invention of the 19th C., which also deployed "resilience" (which is just a result) in the modern sense as a way to cope with being a success or failure.

Result and NonResult, by the way. Is there a third state? REPL has an Incomplete state, a Limbo or perhaps a Bardo.

@sjrd
Copy link
Member

sjrd commented Jan 10, 2019

I am of a mild opinion to keep the status quo. The proposed new situation is not clearly better IMO, so why bother everyone with having to change their code?

@SethTisue
Copy link
Member

SethTisue commented Jan 10, 2019

the big win here would be to ultimately get rid of scala.util entirely, as outlined at scala/scala-dev#323 and #5677, so this PR should be judged in that context

@sjrd
Copy link
Member

sjrd commented Jan 10, 2019

Meanwhile, we just added Using in there ...

@Ichoran
Copy link
Contributor

Ichoran commented Jan 10, 2019

Using can live wherever ControlThrowable and NonFatal live; we don't need to preserve util just for it.

@som-snytt
Copy link
Contributor

ChainingOps was added to chain us to the util package object.

@sjrd
Copy link
Member

sjrd commented Jan 10, 2019

@Ichoran ControlThrowable and NonFatal live in scala.util.control._.

@Ichoran
Copy link
Contributor

Ichoran commented Jan 10, 2019

@sjrd - Yes, but wherever they go, if we get rid of util, Using can go too.

@sjrd
Copy link
Member

sjrd commented Jan 10, 2019

Yes sure. In the meantime, I believe this is not something we should do piecewise. If the justification for moving one thing is that the eventual goal is to get rid of util, then we should move everything somewhere else in one step, or none at all. Otherwise, if later we don't manage to move the other stuff somewhere else for some reason, then we've bothered people with code changes for absolutely no reason.

@som-snytt
Copy link
Contributor

Basically anything that sounds like it's related to bondage should go in a different package.

Also, Using should have a friendlier alias for anyone trying to stay clean.

1. Move `Success` and `Failure` inside the `Try`'s companion object,
2.Add alias for to `scala.util`'s package object and deprecated them in favor of `Scala.Try`, `Try.Success` and `Try.Failure`.
@He-Pin
Copy link
Contributor Author

He-Pin commented Jan 12, 2019

For Try.Success and Try.Failure I think that's fine, The main reason I think is user may defining their own Success and Failure.
image

Rebased, btw.

@NthPortal
Copy link
Contributor

Honestly, I'm not sure that we should get rid of util - there are a lot of miscellaneous small utilities that don't need their own packages but belong in the stdlib. As long as we're not dumping everything in util (as Java did)—which we're not—I think it's fine to have some miscellaneous classes in there.

The question of whether or not Try and Either ought to have been originally put in scala rather than scala.util, as well as whether or not Left, Right, Success and Failure ought to have been nested within their parents, is interesting, but at this point I think I will agree with Sébastien that it's not worth the breakage.

@SethTisue
Copy link
Member

SethTisue commented Jan 29, 2019

I'm convinced (by Seb and Nth) that this should wait for 2.14 and be part of a package of related changes rather happening independently.

@hepin1989 thanks for your work on this, hopefully we can pick this up again later in some form

@SethTisue SethTisue added the WIP label Jan 29, 2019
@SethTisue SethTisue removed this from the 2.13.0-RC1 milestone Jan 29, 2019
@He-Pin
Copy link
Contributor Author

He-Pin commented Jan 30, 2019

@SethTisue I am totally fine with it:)

@SethTisue SethTisue removed their assignment Jan 31, 2019
@lrytz lrytz closed this Mar 5, 2019
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.

8 participants