Skip to content

Conversation

kunaljani1100
Copy link
Contributor

Motivation:

For the Future class, the following functions have been defined repeatedly with multiple definitions accepting multiple number of arguments.

  1. Future.all()
  2. Future.any()
  3. Future.join()

For all the 3 methods, there are multiple definitions with multiple number of arguments.

For example, static CompositeFuture all(Future<?> f1, Future<?> f2), static CompositeFuture all(Future<?> f1, Future<?> f2, Future<?> f3), static CompositeFuture all(Future<?> f1, Future<?> f2, Future<?> f3, Future<?> f4), static CompositeFuture all(Future<?> f1, Future<?> f2, Future<?> f3, Future<?> f4, Future<?> f5), and static CompositeFuture all(Future<?> f1, Future<?> f2, Future<?> f3, Future<?> f4, Future<?> f5, Future<?> f6) are unnecessary declarations of the Future.all() method and can be replaced with a single method that looks like this:

static CompositeFuture all(Future<?> ... futures)

This single function is capable of accepting a variable number of futures at once and will eliminate the need for unnecessary method overloading with multiple number of variables.

Conformance:

You should have signed the Eclipse Contributor Agreement as explained in https://github.com/eclipse/vert.x/blob/master/CONTRIBUTING.md

I have signed the eclipse contributor agreement.

Please also make sure you adhere to the code style guidelines: https://github.com/vert-x3/wiki/wiki/Vert.x-code-style-guidelines

I am adhering to the code style guidelines.

@vietj
Copy link
Member

vietj commented Sep 1, 2025

@kunaljani1100 can you add a test for this ?

@vietj vietj added this to the 5.1.0 milestone Sep 1, 2025
@kunaljani1100 kunaljani1100 reopened this Sep 1, 2025
@kunaljani1100
Copy link
Contributor Author

Hi @vietj, I have added the success as well as the failures conditions for the all(), any(), and join() methods in the FutureTest class.

@kunaljani1100
Copy link
Contributor Author

Currently, the tests have been commented out for right now to find out which tests are the cause of failure. I tested the tests locally and they were working fine for me, but still working on finding out the root cause behind the test failues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants