I agree with Chesnay, that Optional is not so perfect especially in places other than return value.
Some other problems I could see:
1. Optional is a generic type, and generic types not always behave well with
the TypeInformation due to type erasure.
2. Optionals due not work well with Reflection
3. Using Optional as an argument creates lots of boilerplate code:
If we have a method like:
public void myMethod(String arg1, Optonal<String> arg2);
We would need to call it:
myMethod(arg1, Optional.ofNullable(arg2));
myMethod(arg1, Optional.empty());
Instead I think a better solution is just overloading:
public void myMethod(String arg1);
public void myMethod(String arg1, String arg2);
Therefore I think Optional fits only in cases where we want explicitly tell
that a method may not return any value.
> On 25 Aug 2017, at 10:10, Chesnay Schepler <ches...@apache.org> wrote:
>
> I think you're overselling optional a little bit.
>
> The unfortunate fact is that any Optional /can still be null itself/.
>
> As such, if there a method accepts an Optional it still has to check whether
> it is null, /and in addition whether it contains a null.
> /i.e.
>
> public void myMethod(Optional<String> parameter) {
> if (parameter != null) {
> if (parameter.isPresent()) {
> <do something with it>
> }
> }
> }
>
> Effectively this just introduces redundancy, albeit with some syntactic sugar
> on top.
> The same applies for Optional fields.
>
> As for your example as to how it is "/easier to maintain the ban on null",/
> since passing null is just as valid if
> the parameter was an optional wouldn't I still have to go to the
> implementation to check it?
>
> On 24.08.2017 16:58, Piotr Nowojski wrote:
>> Hi,
>>
>> Since we switched to Java 8, I would like to start a discussion about a
>> deprecating usage of @Nullable parameters and return values in Flink and
>> enforcing strict ban on nulls in those places. I just think that having a
>> very explicit way, enforced by a static type checking mechanism of optional
>> values is just so much better and safer compared to constantly thinking if
>> the value that you have can be a null or it can not.
>>
>> Personally I would go even one step further and I would ban nulls as
>> fields/local variables and always use in those cases Optional, with a very
>> rare exceptions for performance reasons. This however has two drawbacks:
>>
>> 1. Optional was not intended by the Java committee to be used on fields. I
>> don’t why, because for me it is just better compared to handling nulls.
>> 2. Optional is not serializable (that’s the collateral damage caused by 1.).
>> Thus in many places for fields we would have to use our own custom
>> OptionalSerializable.
>>
>> Regardless of using Optional for a fields as well, transition from Nullable
>> to Optional should be fairly simple. We could go module by module
>> deprecating old methods taking @Nullable and replacing them with overloaded
>> Optional versions. The only issue is that in the short term it could confuse
>> a little bit users, if we present them two versions of methods and with
>> inconsistency between modules. However I think it’s worth it in the long
>> term.
>>
>> As a side note it is also easier to maintain the ban on nulls, compared to
>> putting @Nullable annotation in appropriate places. With Optional, reviewer
>> could always immediately spot each usage of “null". With @Nullable, if new
>> pull request adds some invocation of a method:
>>
>> foo.bar(arg1, null, arg3);
>>
>> It is impossible to know whether it is allowed, without going to the
>> implementation of the method and checking for presence or lack of the
>> annotation. Thus we have some places in the code where there are
>> undocumented/not annotated @Nullable fields/parameters (even in the public
>> api).
>>
>> What do you think?
>>
>> Piotrek
>
>
signature.asc
Description: Message signed with OpenPGP
