Skip to content

Provide monadic conversions for StatusReply #32238

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

Open
jroper wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Provide monadic conversions for StatusReply #32238

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 15 additions & 0 deletions akka-actor/src/main/scala/akka/pattern/StatusReply.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,21 @@ final class StatusReply[+T] private (private val status: Try[T]) {
def isError: Boolean = status.isFailure
def isSuccess: Boolean = status.isSuccess

/**
* Return this StatusReply as a Try.
*/
def toTry: Try[T] = status
Copy link
Contributor

@patriknw patriknw Nov 24, 2023

Choose a reason for hiding this comment

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

No problem with adding those. Normally StatusReply is supposed to be used with askWithStatus and the returned Future is already flattened to Future[T]. I guess you are using it without askWithStatus?

Copy link
Contributor Author

@jroper jroper Nov 27, 2023

Choose a reason for hiding this comment

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

Well, I was, I was refactoring some code that was using it, and found myself wanting to convert to either/try. But then as I expanded the scope of the refactoring, I found there was no good reason for it to be using a Future[StatusReply[T]] in the first place, that whatever errors it had were eventually flattened into a Future anyway, so I modified it to use askWithStatus. So, I don't actually have a use case for this now.

I think I would have more of a use case for map and flatMap on it, in the actor behaviour, the code I was working with was passing a lot of StatusReply types around when generating a reply, and although I didn't need these methods, I could see that they might be potentially useful. Though maybe the actor should have just worked with Try. But anyway, thats where toTrymight be useful, to do amapbefore converting back toStatusReply`.

Copy link
Contributor

@patriknw patriknw Dec 7, 2023

Choose a reason for hiding this comment

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

Then I suggest that we make this minimal by only adding toTry.

Java should already be covered by existing methods.


/**
* Return this StatusReply as an Either.
*/
def toEither: Either[Throwable, T] = status.toEither

/**
* Return this StatusReply as an Option, returning None and discarding the error if it's an Error.
*/
def toOption: Option[T] = status.toOption
Copy link
Contributor

@patriknw patriknw Nov 24, 2023

Choose a reason for hiding this comment

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

Add Scala API: to the docs of all 3 methods:

/**
   * Scala API: Return this StatusReply as an Option, returning None and discarding the error if it's an Error.
   */

Also add a def toOptional for Java.


override def equals(other: Any): Boolean = other match {
case that: StatusReply[_] => status == that.status
case _ => false
Expand Down