linkedin/parseq

andThen overloading is confusing

junchuanwang opened this issue · 4 comments

We have two andThen function available. One takes a consumer and second takes a Task.
They have different semantics and behavior.

https://github.com/linkedin/parseq/wiki/User's-Guide#sequential-composition

A misuse case is that user might use the variant that takes a Consumer function to return the task and want that task to be run, because any object can be cast to void, so the compiler won't issue warning of the consumer function returns the task

Solution can be
(1) add a new interface, e.g. something like andThenTask
(2) add additional logic to see if the return value from the Consumer function is of Task type and execute it (this requires another overloading andThen to take a Function<T, Task>

Linked Issue:
#207

+1 andThen use be used to indicate order of execution.
And the fact that when it takes a Task but does not pass on the result from the previous task is unexpected.

#291 can be complementary solution to #287 and #207

Good on identifying the misuse. +1 for slight preference on the resolution (2)
But I feel there is a common understanding among what consumer means, so it can return a task sounds a bit not common and different, so I suggest never expect consumer return anything

option (2) is not viable: the Consumer and Function when they apply to the lambda function, they have same erased type. So the lambda function will find two ambiguous overloading.

A better options is to change the signature to
default Task<T> andThen(final String desc, final Function1<? super T, ?> function)
and
check the return valuke of the function call.

Why I changed my mind:
But same behavior is achievable using flatMap. So it is hard to say if want to run the task if the return type found to be a task, because the user got this confusion just because they probably not aware of FlatMap.

I think we should educate the users instead.