Skip to content

Conversation

@julianladisch
Copy link
Contributor

@julianladisch julianladisch commented Jan 3, 2025

Motivation:

Simplify the code by replacing onComplete + if-ar.succeeded-else with
either onComplete(res, e) or with other processing.

Avoiding onComplete + if-ar.succeeded-else results in more concise and understandable
code because it avoids the additional nesting of the if-else clause.

Extend the exampleFuture* code to discuss all possibilities how onComplete and
onSuccess/onFailure might be used.

Conformance:

@julianladisch julianladisch force-pushed the CoreExamples-onSuccess-onFailure branch from 0fb18e0 to 3aaa5d6 Compare January 3, 2025 22:40
Copy link
Member

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

Thanks @julianladisch for bringing this up.

I have reservations about this one. Invoking onSuccess and onFailure at the same place implies creating two future listeners where a single one would do the trick.

If the goal is to avoid writing the if block, then there is an onComplete overload that takes two arguments, a success and a failure handler.

We should promote the usage of this method, imo.

That doesn't mean onSucess and onFailure don't have any use case (sometimes they're convenient, like when a method returning a future needs to perform a side action when the future is completed).

@julianladisch julianladisch force-pushed the CoreExamples-onSuccess-onFailure branch from 3aaa5d6 to 09fde45 Compare January 8, 2025 23:08
@julianladisch julianladisch changed the title Replace onComplete with onSuccess + onFailure Jan 8, 2025
@julianladisch julianladisch force-pushed the CoreExamples-onSuccess-onFailure branch from 09fde45 to d5a5ca5 Compare January 8, 2025 23:20
Simplify the code by replacing onComplete + if-ar.succeeded-else with
either onComplete(res, e) or with other processing.

Avoiding onComplete + if-ar.succeeded-else results in more concise and understandable
code because it avoids the additional nesting of the if-else clause.

Extend the `exampleFuture*` code to discuss all possibilities how onComplete and
onSuccess/onFailure might be used.

Remove unused exampleFuture2 from lines 173-186. A new exampleFuture2 is created in line 145.
@julianladisch julianladisch force-pushed the CoreExamples-onSuccess-onFailure branch from d5a5ca5 to e9fe56f Compare January 8, 2025 23:26
@julianladisch
Copy link
Contributor Author

@tsegismont @vietj : Thank you for pointing out that invoking onSuccess and onFailure at the same place is slightly slower and takes slightly more memory. I've adjusted the changes accordingly.
Please re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants