linkedin/parseq

Introduce `Task.fromTry` to convert `Try` to `Task` with success/failure semantics

zackthehuman opened this issue · 5 comments

I sometimes find myself performing a conversion from a Try<T> to a Task<T> where a failed Try would generate a failed Task and a successful Try would generate a completed Task containing the value.

The opposite conversion exists as part of the public API of Task via Task.toTry.

Is there any reason not to provide a conversion from Try to Task? If we're open to it I would be happy to send a PR with the addition.

@zackthehuman I don't think there is a reason not to include this feature. You can go ahead with the PR for this.
I'd prefer this to be added on Try (as Try::toTask).

@junchuanwang @aman1309 FYI

@karthikbalasub Sounds good. I have been thinking about how the API should be expressed and want to get some feedback before I dive in. In my projects I have been using a static conversion method as a workaround for this API not being present, but there are a few ways to go about it if it were part of the library.

Static vs instance method

My initial thought was to implement Try::toTask as a static method the same way I have done in the utility in my project. However, Task::toTry is an instance method, which totally makes sense. I could do the same and add Try::toTask to Try's interface and update Success and Failure accordingly. There is a nice symmetry here between the two classes. However, following that path lead me to another challenge...

Task knows about Try, but Try doesn't know about Task

The Try interface and classes are very simple and are totally ignorant of Task. Introducing a conversion to Task will change this ignorance a bit -- which feels a bit odd when looking at how the packages are designed. This wasn't a problem when using a utility class to perform the conversion since it has no impact on either interface.

Adding Try::toTask

Pros

  • Symmetric with Task::toTry
  • Conversion is always done via instance method

Cons

  • Introduces new coupling between Try (and its implementations) and Task

Adding static Task::fromTry

Pros

  • Similar to existing methods Task::value, Task::failure, Task::fromCompletionStage, etc.
  • Maintains contract that Task knows about Try, but Try doesn't know about Task

Cons

  • Asymmetric between to and from (Task::toTry is an instance method, Task::fromTry would be a static)

My proposal would be to add Task::fromTry as a static method of Task since it is the least disruptive to the API and serves as another factory method for Task. What are your thoughts?

@zackthehuman Sorry for the delayed response, I missed your reply..
Thanks for the detailed mail with the options. I'm okay with your proposal. It is also consistent with Task::fromCompletionStage.

@junchuanwang FYI

I also lean towards the Task::fromTry solution. Thanks for implementing this

@karthikbalasub Sounds good. I have been thinking about how the API should be expressed and want to get some feedback before I dive in. In my projects I have been using a static conversion method as a workaround for this API not being present, but there are a few ways to go about it if it were part of the library.

Static vs instance method

My initial thought was to implement Try::toTask as a static method the same way I have done in the utility in my project. However, Task::toTry is an instance method, which totally makes sense. I could do the same and add Try::toTask to Try's interface and update Success and Failure accordingly. There is a nice symmetry here between the two classes. However, following that path lead me to another challenge...

Task knows about Try, but Try doesn't know about Task

The Try interface and classes are very simple and are totally ignorant of Task. Introducing a conversion to Task will change this ignorance a bit -- which feels a bit odd when looking at how the packages are designed. This wasn't a problem when using a utility class to perform the conversion since it has no impact on either interface.

Adding Try::toTask

Pros

  • Symmetric with Task::toTry
  • Conversion is always done via instance method

Cons

  • Introduces new coupling between Try (and its implementations) and Task

Adding static Task::fromTry

Pros

  • Similar to existing methods Task::value, Task::failure, Task::fromCompletionStage, etc.
  • Maintains contract that Task knows about Try, but Try doesn't know about Task

Cons

  • Asymmetric between to and from (Task::toTry is an instance method, Task::fromTry would be a static)

My proposal would be to add Task::fromTry as a static method of Task since it is the least disruptive to the API and serves as another factory method for Task. What are your thoughts?

+1 on this to avoid introducing Task into Try interface.