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

zackthehuman commented 4 years ago

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.

karthikbalasub commented 4 years ago

@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

zackthehuman commented 4 years ago

@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?

karthikbalasub commented 4 years ago

@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

junchuanwang commented 4 years ago

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

mchen07 commented 4 years ago

@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.

linkedin / parseq