EDIT: for Optional in public API vs performance concerns
Hi all,
This is the next follow up discussion about suggestions for the recent
thread about code style guide in Flink [1].
In general, one could argue that any variable, which is nullable, can be
replaced by wrapping it with Optional to explicitly show that it can be
null. Examples are:
- returned values to force user to check not null
- optional function arguments, e.g. with implicit default values
- even class fields as e.g. optional config options with implicit
default values
At the same time, we also have @Nullable annotation to express this
intention.
Also, when the class Optional was introduced, Oracle posted a guideline
about its usage [2]. Basically, it suggests to use it mostly in APIs for
returned values to inform and force users to check the returned value
instead of returning null and avoid NullPointerException.
Wrapping with Optional also comes with the performance overhead.
Following the Oracle's guide in general, the suggestion is:
- Always use Optional only to return nullable values in the API/public
methods
- Only if you can prove that Optional usage would lead to a
performance degradation in critical code then return nullable value
directly and annotate it with @Nullable
- Passing an Optional argument to a method can be allowed if it is
within a private helper method and simplifies the code, example is in [3]
- Optional should not be used for class fields
Please, feel free to share you thoughts.
Best,
Andrey
[1]
http://mail-archives.apache.org/mod_mbox/flink-dev/201906.mbox/%3ced91df4b-7cab-4547-a430-85bc710fd...@apache.org%3E
[2]
https://www.oracle.com/technetwork/articles/java/java8-optional-2175753.html
[3]
https://github.com/apache/flink/blob/master/flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/typeutils/AvroFactory.java#L95
On Thu, Aug 1, 2019 at 6:00 PM Andrey Zagrebin <and...@ververica.com> wrote:
> Hi all,
>
> This is the next follow up discussion about suggestions for the recent
> thread about code style guide in Flink [1].
>
> In general, one could argue that any variable, which is nullable, can be
> replaced by wrapping it with Optional to explicitly show that it can be
> null. Examples are:
>
> - returned values to force user to check not null
> - optional function arguments, e.g. with implicit default values
> - even class fields as e.g. optional config options with implicit
> default values
>
>
> At the same time, we also have @Nullable annotation to express this
> intention.
>
> Also, when the class Optional was introduced, Oracle posted a guideline
> about its usage [2]. Basically, it suggests to use it mostly in APIs for
> returned values to inform and force users to check the returned value
> instead of returning null and avoid NullPointerException.
>
> Wrapping with Optional also comes with the performance overhead.
>
> Following the Oracle's guide in general, the suggestion is:
>
> - Avoid using Optional in any performance critical code
> - Use Optional only to return nullable values in the API/public
> methods unless it is performance critical then rather use @Nullable
> - Passing an Optional argument to a method can be allowed if it is
> within a private helper method and simplifies the code, example is in [3]
> - Optional should not be used for class fields
>
>
> Please, feel free to share you thoughts.
>
> Best,
> Andrey
>
> [1]
> http://mail-archives.apache.org/mod_mbox/flink-dev/201906.mbox/%3ced91df4b-7cab-4547-a430-85bc710fd...@apache.org%3E
> [2]
> https://www.oracle.com/technetwork/articles/java/java8-optional-2175753.html
> [3]
> https://github.com/apache/flink/blob/master/flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/typeutils/AvroFactory.java#L95
>
