Hi, First, Optional is just a wrapper, just like boxed value. So as long as it's not a field level operation, I think it is OK to performance. I think guava optional has a good summary to the uses. [1] > As a method return type, as an alternative to returning null to indicate that no value was available > To distinguish between "unknown" (for example, not present in a map) and "known to have no value" > To wrap nullable references for storage in a collection that does not support The latter two points seem reasonable, but they have few scenes.
[1] https://github.com/google/guava/blob/master/guava/src/com/google/common/base/Optional.java Best, Jingsong Lee ------------------------------------------------------------------ From:Timo Walther <twal...@apache.org> Send Time:2019年8月2日(星期五) 14:12 To:dev <dev@flink.apache.org> Subject:Re: [DISCUSS][CODE STYLE] Usage of Java Optional Hi everyone, I would vote for using Optional only as method return type for non-performance critical code. Nothing more. No fields, no method parameters. Method parameters can be overloaded and internally a class can work with nulls and @Nullable. Optional is meant for API method return types and I think we should not abuse it and spam the code with `@SuppressWarnings("OptionalUsedAsFieldOrParameterType")`. Regards, Timo Am 02.08.19 um 11:08 schrieb Biao Liu: > Hi Jark & Zili, > > I thought it means "Optional should not be used for class fields". However > now I get a bit confused about the edited version. > > Anyway +1 to "Optional should not be used for class fields" > > Thanks, > Biao /'bɪ.aʊ/ > > > > On Fri, Aug 2, 2019 at 5:00 PM Zili Chen <wander4...@gmail.com> wrote: > >> Hi Jark, >> >> Follow your opinion, for class field, we can make >> use of @Nullable/@Nonnull annotation or Flink's >> SerializableOptional. It would be sufficient. >> >> Best, >> tison. >> >> >> Jark Wu <imj...@gmail.com> 于2019年8月2日周五 下午4:57写道： >> >>> Hi Andrey, >>> >>> I have some concern on point (3) "even class fields as e.g. optional >> config >>> options with implicit default values". >>> >>> Regarding to the Oracle's guide (4) "Optional should not be used for >> class >>> fields". >>> And IntelliJ IDEA also report warnings if a class field is Optional, >>> because Optional is not serializable. >>> >>> >>> Do we allow Optional as class field only if the class is not serializable >>> or forbid this totally? >>> >>> Thanks, >>> Jark >>> >>> On Fri, 2 Aug 2019 at 16:30, Biao Liu <mmyy1...@gmail.com> wrote: >>> >>>> Hi Andrey, >>>> >>>> Thanks for working on this. >>>> >>>> +1 it's clear and acceptable for me. >>>> >>>> To Qi, >>>> >>>> IMO the most performance critical codes are "per record" code path. We >>>> should definitely avoid Optional there. For your concern, it's "per >>> buffer" >>>> code path which seems to be acceptable with Optional. >>>> >>>> Just one more question, is there any other code paths which are also >>>> critical? I think we'd better note that clearly. >>>> >>>> Thanks, >>>> Biao /'bɪ.aʊ/ >>>> >>>> >>>> >>>> On Fri, Aug 2, 2019 at 11:08 AM qi luo <luoqi...@gmail.com> wrote: >>>> >>>>> Agree that using Optional will improve code robustness. However we’re >>>>> hesitating to use Optional in data intensive operations. >>>>> >>>>> For example, SingleInputGate is already creating Optional for every >>>>> BufferOrEvent in getNextBufferOrEvent(). How much performance gain >>> would >>>> we >>>>> get if it’s replaced by null check? >>>>> >>>>> Regards, >>>>> Qi >>>>> >>>>>> On Aug 1, 2019, at 11: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 >>>>>
