Thanks Jan for bringing this subject to the mailing list. I tend to lean towards the java Optional. Optionals are mostly useful as return types and since return types usually end up in the API. I am afraid we end up leaking Guava's Optional as part of the public API. We can define however a rule to not allow Optional in the public APIs (and enforce it via checkstyle) and in that case it will be ok to use either of them.
Ismaël On Thu, Aug 22, 2019 at 1:29 AM Kenneth Knowles <[email protected]> wrote: > > As mentioned on PR, I'm not convinced by Flink's discussion and all evidence > I know has shown it to have non-measurable performance impact. > > I'm OK either way, though, at this point. > > - Whatever the consensus, let us set up checkstyle/analysis so that we > maintain compatible across the codebase. > - We should see what we can do to enforce not using non-serializable > Optional in fields. > - In special cases where you really do want an Optional stored or > transmitted we could make our own SerializableOptional to still keep it > compatible (example: the common Map<K, Optional<V>> representing the three > states of --foo=v --foo=null and no foo specified) > - Always using null & @Nullable for possible-empty fields will make the > codebase poorer overall, and NPEs are still a user-facing problem that hits > us pretty often. We should redouble our efforts to have a correct and fully > strict analysis of these. > > Kenn > > On Wed, Aug 21, 2019 at 1:09 PM Jan Lukavský <[email protected]> wrote: >> >> Sorry, forgot to add link to the Flink discussion [1]. >> >> [1] >> https://lists.apache.org/thread.html/f5f8ce92f94c9be6774340fbd7ae5e4afe07386b6765ad3cfb13aec0@%3Cdev.flink.apache.org%3E >> >> On 8/21/19 10:08 PM, Jan Lukavský wrote: >> > Hi, >> > >> > sorry if this discussion have been already taken, but I'd like to know >> > others opinions about how we use Optionals. The state in current >> > master is as follows: >> > >> > $ git grep "import" | grep "java.util.Optional" | wc -l >> > 85 >> > $ git grep "import" | grep "Optional" | grep guava | wc -l >> > 45 >> > >> > I'd like to propose that we use only one Optional, for consistency. >> > There are arguments towards using each one of them, if I try to sum >> > these up: >> > >> > Pros for java.util: >> > >> > * Part of standard lib >> > >> > * Will (in the future) probably better integrate with other standard >> > APIs (e.g. Optional.stream in JDK 9, but probably more to come) >> > >> > Pros for guava: >> > >> > * Guava's Optional is Serializable >> > >> > There was recently a discussion on Flink's mailing list [1], which >> > arrived at a conclusion, that using Optional as a field should be >> > discouraged (in favor of @Nullable). That would imply that >> > non-serializability of java.util.Optional is not an issue. But maybe >> > we can arrive at a different conclusion. >> > >> > WDYT? >> > >> > Jan >> >
