Hi Andrey, I think that’s a good compromise easy to follow, so +1 from my side.
Piotrek > On 1 Aug 2019, at 18:00, Andrey Zagrebin <and...@ververica.com> wrote: > > 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 >>
