Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FailsafeCall micro refactoring, plus World-Wide Nr.1 duplicated utility method for OkHttp #351

Open
magicprinc opened this issue Nov 26, 2022 · 4 comments

Comments

@magicprinc
Copy link

magicprinc commented Nov 26, 2022

As you can see here:
magicprinc@c517e3e

FailsafeCall micro refactoring:

  1. AtomicBoolean fields are final

  2. lambda expression instead of code block

  3. World-Wide Nr.1 duplicated utility method for OkHttp
    `/** [OkHttp Callback to JDK CompletableFuture]

    Helps eliminate dozens of utility classes World-wide with exactly this same method.

    Can be the first small step towards FailSafe.

    Returns normal JDK {@link CompletableFuture} without FailSafe policies.
    */

public static CompletableFuture asPromise (okhttp3.Call call)`

All around the World, people write this method again and again. I have done it too.
We really need "The Chosen One". I recommend you to be this one :-)

If you like it, I will send it as PR.

@Tembrel
Copy link
Contributor

Tembrel commented Nov 28, 2022

  1. AtomicBoolean fields are final

OK

  1. lambda expression instead of code block

Aren't those braces ignored, anyway? It's just a single expression. (The indenting of the modified code is off, btw.)

All around the World, people write this method again and again. I have done it too. We really need "The Chosen One". I recommend you to be this one :-)

Does this utility belong in FailsafeCall.java? If included at all, wouldn't it be better as a single static method in a utility class?

@magicprinc
Copy link
Author

magicprinc commented Nov 28, 2022

Aren't those braces ignored, anyway? It's just a single expression. (The indenting of the modified code is off, btw.)

IntelliJ IDEA's work :-)

As I know, the resulting byte code is the same.

Does this utility belong in FailsafeCall.java? If included at all, wouldn't it be better as a single static method in a utility class?

To be honest, not 100%... but I really want to drop such "one method utility class" in my codebase.
And I don't like to add new dependencies. Failsafe have managed after long testing, etc.

🙏

If you do some research in Internet, you will find that people reinvent this wheel over and over, write almost the same method over and over.
We must protect the Earth from global warming! :-)

As a small sign of gratitude, I won't then write an issue about a large PR to concurrentunit (Excellent library BTW)

@magicprinc
Copy link
Author

Your wish is my command. Tell me if I should send an PR.

@Tembrel
Copy link
Contributor

Tembrel commented Nov 28, 2022

Not up to me.

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

No branches or pull requests

2 participants