iluwatar / java-design-patterns

Design patterns implemented in Java
https://java-design-patterns.com
Other
88.32k stars 26.21k forks source link

Incorrect use of Optional class in Async Method Invocation #2829

Closed iluwatar closed 3 months ago

iluwatar commented 3 months ago

Optional class is used incorrectly in Async Method Invocation pattern.

The Optional class in Java is primarily intended to provide a type-level solution for representing optional values, instead of using null. It's a container object that may or may not contain a non-null value. However, using Optional as a field in a class or as a parameter in a method is not recommended for several reasons:

Instead of using Optional as a field, it's often better to use a nullable field and add a comment that the field can be null. For method parameters, consider overloading the method or using a builder pattern if a parameter is optional.

Acceptance criteria

guqing commented 3 months ago

Hi @iluwatar,

May I ask if I can solve this problem? Based on your description, my solution is roughly as follows:

  1. Method Overloading: We can overload the onComplete method to have two distinct methods - one for handling successful execution and another for handling exceptions. This approach would eliminate the need for Optional and clearly separate the success and failure handling paths.

    public interface AsyncCallback<T> {
    void onComplete(T value);
    
    // or other method names: onFailed / onThrowable 
    void onError(Exception ex);
    }
  2. Nullable Parameters: As an alternative, we could modify the onComplete method to accept nullable parameters, clearly documenting that the exception parameter can be null in case of success and vice versa.

    public interface AsyncCallback<T> {
    void onComplete(T value, @Nullable Exception ex);
    }

If this sounds good, I'd be more than happy to take on this task and submit a pull request with the necessary changes. Please let me know if I can go ahead with this approach or if there are any other preferences or suggestions.

iluwatar commented 3 months ago

@guqing both sound viable options. I'm happy to review the PR whichever solution you end up recommending.