Incorrect use of Optional class in Async Method Invocation
iluwatar opened this issue · 2 comments
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:
-
Serialization: Optional is not serializable, so classes with Optional fields would not be serializable either. This can lead to problems if you're trying to send objects across a network or save them in a file for later use.
-
Increased complexity: Using Optional in these ways can make the code more complex and harder to understand. It's not immediately clear, for example, what it means for an Optional to be present or absent in the context of a field or method parameter.
-
Performance overhead: Optional creates an extra object for every wrapped value, which can lead to unnecessary memory usage and performance overhead.
-
Not intended use: The intended use of Optional is as a return type. The Java API designer, Brian Goetz, stated that Optional was designed to provide a limited mechanism for library method return types where there is a clear need to represent "no result", and using it as a field type is not recommended.
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
- Optional class fields and method parameters refactored
Hi @iluwatar,
May I ask if I can solve this problem? Based on your description, my solution is roughly as follows:
- 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);
}- 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.