Lombok: [FEATURE] Generate Optional getters

Created on 18 Oct 2019  ยท  20Comments  ยท  Source: projectlombok/lombok

Opened from #2263

Describe the feature
Consider this source code:

@Builder.Default
private @Getter final @Nullable String token = null;

What do we get from this code?

  • An optional builder variable token that is null if unset.
  • An @org.jetbrains.annotations.Nullable annotated String field: token
  • A method #getToken(): @Nullable String

Due to the org.jetbrains.annotations.Nullable annotation being present on a final field, we could assume that the desired getter should be generated to #getToken(): Optional<String>.
However, there is no option to differentiate what kind of getter you desire, you always get a Nullable String getter.
This can be overcome with an annotation field useOptional or such to define whether or not Optional wrapping is desired.

Describe the target audience
This change would make lombok especially fitting for a project that makes large use of the Java Stream API.

Most helpful comment

Just my two cents here:

I don't care much for serialization either -- however, I agree with Goetz' guideline for using Optional as a return type, not a field type, because it just makes a lot of sense. When you're writing a class, say Person, the fields of that class (name, age, etc) are best expressed as their respective types, which keeps things clutter-free and simple.

Optional was introduced to, as Goetz puts it, force the caller of a method to actively unwrap its returned object and consider the possibility of no value. Which is why people are using it when designing APIs, because it enforces this contract at the code-level and not with an annotation, which compilers/IDEs may or may not notify the developer of. So while null is just fine for internal implementation, Optional is very helpful for public APIs. "The API itself was unclear about what it returns" can be addressed with javadoc/annotations, but is _better addressed_ with this explicit type.

I noticed in your 'four cases' where you attack the utility of the Optional type that you discount the utility provided by the class' methods, like map, flatMap, and a few others (I too am not a fan of ifPresent, admittedly).

But forget all of this and just consider the following:

While this is your library and you can do whatever, your personal opinion that "it's laughably insane to want it. Optional is for potentially-no-value returning terminals on stream ops. AND NOTHING ELSE." is a bizarre reason why this shouldn't be supported by Lombok -- this opinion of yours is hardly a consensus, not even close. At the very least @Getter.Optional could be an experimental feature to get community feedback. Or if not that, then just tabling it as a possible feature to consider for the future. Not saying your (slightly eccentric) opinion is wrong, but I'm questioning why it and it alone should pose an obstacle to at least seeing how the broader community feels about this.

All 20 comments

Why not simply define that field as an Optional<> instead?

@Getter
@NonNull
@Builder.Default
private final Optional token = Optional.empty();

On Fri, Oct 18, 2019 at 5:26 PM Tobias Burdow [Kaleidox] <
[email protected]> wrote:

Describe the feature
Consider this source code:

@Builder.Defaultprivate @Getter final @Nullable String token = null;

What do we get from this code?

  • An optional builder variable token that is null if unset.
  • An @org.jetbrains.annotations.Nullable annotated String field: token
  • A method #getToken(): @Nullable String

Due to the org.jetbrains.annotations.Nullable annotation being present on
a final field, we could assume that the desired getter should be
generated to #getToken(): Optional.
However, there is no option to differentiate what kind of getter you
desire, you always get a Nullable String getter.
This can be overcome with an annotation field useOptional or such to
define whether or not Optional wrapping is desired.

Describe the target audience
This change would make lombok especially fitting for a project that makes
large use of the Java Stream API.

โ€”
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2264?email_source=notifications&email_token=AABIERI3VSXWKGAVLORJ3MTQPHIRZA5CNFSM4JCI5YR2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HSZBC7A,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERJPMCIPZOMXIOAPIG3QPHIRZANCNFSM4JCI5YRQ
.

--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven

Because storing optionals as field values is stupid af.

I know what Brian Goetz said. I happen to think he is wrong on this
particular subject.

Sure, Optional as it stands wasn't exactly designed for this, but the
concept of a single-object container to force null handling is pretty solid.
Unless you care about serialization (I don't) or have to deal with
frameworks that do not support it (most of them do, nowadays) there really
is no good reason to not use Optionals in fields.
In fact, lombok supports it perfectly. So does Jackson. And Hibernate.

So I ask you, what exactly is so "stupid" about it?

On Thu, Oct 24, 2019 at 12:45 AM Tobias Burdow [Kaleidox] <
[email protected]> wrote:

Because storing optionals as field values is stupid af.

โ€”
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2264?email_source=notifications&email_token=AABIERNWHDKSPS3I7JAF4LTQQDHXBA5CNFSM4JCI5YR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECDDQ2I#issuecomment-545667177,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERPA6YWBRSMPIFG2FWDQQDHXBANCNFSM4JCI5YRQ
.

--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven

Here's an example of an Optional as a field:

https://dev.to/piczmar_0/java-optional-in-class-fields-why-not-40df

On Thu, Oct 24, 2019 at 12:57 AM Floris Kraak randakar@gmail.com wrote:

>

I know what Brian Goetz said. I happen to think he is wrong on this
particular subject.

Sure, Optional as it stands wasn't exactly designed for this, but the
concept of a single-object container to force null handling is pretty solid.
Unless you care about serialization (I don't) or have to deal with
frameworks that do not support it (most of them do, nowadays) there really
is no good reason to not use Optionals in fields.
In fact, lombok supports it perfectly. So does Jackson. And Hibernate.

So I ask you, what exactly is so "stupid" about it?

On Thu, Oct 24, 2019 at 12:45 AM Tobias Burdow [Kaleidox] <
[email protected]> wrote:

Because storing optionals as field values is stupid af.

โ€”
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2264?email_source=notifications&email_token=AABIERNWHDKSPS3I7JAF4LTQQDHXBA5CNFSM4JCI5YR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECDDQ2I#issuecomment-545667177,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERPA6YWBRSMPIFG2FWDQQDHXBANCNFSM4JCI5YRQ
.

--
"Don't only practice your art, but force your way into it's secrets, for
it and knowledge can raise men to the divine."
-- Ludwig von Beethoven

--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven

When it comes to a larger scale, having 10 thousand instances of an object use an instance of Optional instead of simply null, there definetely is memory overhead involved. Talk about why most people hate Java.

Then don't do 10.000 instances of Optional. Use that at the application
boundaries and get rid of it as soon as possible.

As for your remark that "most people hate java" .. that's just an opinion.
Feel free to take that elsewhere, this is neither the time nor place for it.

On Thu, Oct 24, 2019 at 2:03 PM Tobias Burdow [Kaleidox] <
[email protected]> wrote:

When it comes to a larger scale, having 10 thousand instances of an object
use an instance of Optional instead of simply null, there definetely is
memory overhead involved. Talk about why most people hate Java.

โ€”
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2264?email_source=notifications&email_token=AABIERI6LGANTYFUSDRNETTQQGFKNA5CNFSM4JCI5YR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECEY5DY#issuecomment-545885839,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERMIZQSFHX37NAEYIC3QQGFKNANCNFSM4JCI5YRQ
.

--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven

Having a way for a getter to return Optional<T> instead of T might be useful for designing an API without having to wrap/unwrap internally but I don't think memory footprint of empty fields is a valid reason to add it. Optional.empty() returns a singleton. All 10,000 instances will take up the memory foot print of one Optional.empty().

@randakar "then don't do 10k instances" is not a very diplomatic thing to say in this context, as the target of this whole issue is to omit optional fields.......

@ghunteranderson good point, i messed this up, thanks for the reminder.

even after that, using Optionals as field values is dumb IMO and it also is against my style guidance, so, if the artifact i use to reach my style guidance requires me to omit my guidance, i will omit the artifact instead and go for an entirely different stlyle.

If there is a compelling use case with a large enough audience to justify this, it might be Hibernate's support for Optional getters. JPA entities benefit a LOT from @Data or it's effective annotations. They work together so well that I never again want to use JPA without Lombok. Hibernate supports getters that return Optional<T> but since Optional is not serializable, it cannot be used in fields. Consider the vanilla JPA entity below:

@Entity
public class Dog implements Serializable {

    // Mapping to columns with Optional<Human> is not supported.
    // Field owner must be serializable.
    @OneToOne
    private Human owner;

    // Clearly communicates that that not all Dogs will have an owner :(
    public Optional<Owner> getOwner() {
        return Optional.ofNullable(owner);
    }

    public void setOwner(Human owner) {
        this.owner = owner;
    }

}

A Lombok variation might look like this. It adds just a little more flexibility in API design like the fluent and chain attributes in @Accessors.

@Entity
@Data
public class Dog implements Serializable {

    @Getter.Optional // or any other attribute/annotation syntax
    // perhaps @Accessors(optional=true)
    @ManyToOne(fetch = FetchType.LAZY)
    private Human owner;

}

+1, I would like to see this feature, hibernate/jackson being my primary targets. I would say that I can see both sides, Optional as a field is very useful for writing builders by hand.

Optional? No. Just no. It's a horrible idea. You don't 'fix' nulls by introducing a concept that is API-incompatible with 25 years of third party libraries on the most popular language on the planet, it's laughably insane to want it.

Optional is for potentially-no-value returning terminals on stream ops. __AND NOTHING ELSE__.

You could still add it as an additional argument, and your highly unprofessional response shows that I don't wanna be using Lombok any time in the future. ๐Ÿš€

Edit: You're an Idiot for using Nullable-getters in 2020. ๐Ÿ™‚

Yet it is those "third party libraries" that are inexonerably introducing
Optional as a supported language feature. JPA returns optionals from find()
methods, Jackson supports Optional fields, Hibernate has been taught how to
deal with it, etc etc.

As things stand with a modern Spring Boot application you can use Optional
fields in model and dto objects without much of an issue.

So far I have seen one, and only one good reason to not want Optional
fields: It does not implement Serializable.
So, the "backwards compatibility" argument, in a nutshell.

Everything else either prioritizes performance over correctness, or rants
about "it not being designed for this" -> the "Serializable" argument
again, or quotes one of the java language designers who made that argument
and then expect the discussion to go away.

Let me ask you: How else is a serious application developer going to
protect himself against NullPointer exceptions?
And please understand: This is a "billion dollar mistake" in language
design. It is by far the most occurring problem in java applications, full
stop. There are numbers to back that up too.

Right now, if you want to deal with fields that may be empty, you have the
following options:

  • Null check things religiously, hope nothing breaks.
  • Use NonNullApi / Nullable / NotNull validation annotations, rely on your
    IDE and Sonar warnings
  • Wrap fields inside Optionals, let the compiler break the build.

The first option has been tried for years. It has made NullPointerException
the number one bug on the planet. It obviously isn't working.
The second option depends on good tooling and SONAR usage. It's possible to
work that way, but changing annotations may silently break things elsewhere
meaning you won't see it until SONAR complains. If your team even
integrated SONAR properly, which it may not have.
The third option provides fail fast and makes your compiler guarantee
correctness, at the expense of some performance and not being able to use
java serialization. Except that even that still keeps on working, at least
for JMS, so ... what exactly was that backwards compatibility problem again?

On Wed, Jan 15, 2020 at 6:13 PM Reinier Zwitserloot <
[email protected]> wrote:

Optional? No. Just no. It's a horrible idea. You don't 'fix' nulls by
introducing a concept that is API-incompatible with 25 years of third party
libraries on the most popular language on the planet, it's laughably insane
to want it.

Optional is for potentially-no-value returning terminals on stream ops. AND
NOTHING ELSE
.

โ€”
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2264?email_source=notifications&email_token=AABIERIHWWNBLHYQIUOIVVDQ54727A5CNFSM4JCI5YR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJBCNIY#issuecomment-574760611,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERKBWMPVMMSQB5BUCFDQ54727ANCNFSM4JCI5YRQ
.

--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven

How else do you protect against null pointer exceptions? The same way you protect against any other bug. Tests, and writing better. Note that NPEs, if they occur, mean there is some sort of misunderstanding. Either:

  • The API itself was unclear about what it returns, and some API user as a consequence did not realize it could return null.
  • The API was crystal clear, but the API user didn't read the docs or otherwise didn't follow instructions.
  • The API was crystal clear, and the user understood, but the user was under the mistaken impression that whilst the API was specced to perhaps return null, the user didn't think that could happen.
  • A bug in the code: The API says it can't return null, but it does anyway.

There are no other options.

Let's go through em:

  • API unclear: Well, fix that. With an annotation, such as @NonNull. This is backwards compatible (you can take existing API, such as for example j.u.Map's .get() method, and annotate it. You cannot replace its return value with Optional<V>, which is why optional is bad and should not be used for this. It's inferior.
  • API clear, user didn't read: Same answer. An annotation will solve the problem. If the user is unwilling to look at that annotation and unwilling to use their tooling's support for it, that's on the user, no longer on the API designer.
  • API clear, user clear, but user has a mental model error: Optional is worse than NPE, by a considerable margin: If you use optional.ifPresent to map the value (because then the bug would appear as: The code silently does nothing. Even if you use the more appropriate optional.get(), then your code is just.. longer. in normal java world, you get an NPE, pointing right at your mistake. That's better than any other alternative.
  • API clear, user clear, API is bugged: A bug is a bug. Optional does not help here. It is possible the bug could have been avoided by having the API author be more careful with their no-value handling, which is a recursive application of this list.

There is no upside to using Optional in your returns over an annotation.

Tests, and writing better, haven't reduced this problem to something less
than the number 1 bug in java code in the past 20 odd years.

Furthermore, I do not care about the cases where something really never
should be empty. Nonnull handles that fine. Ditto for NotNull. No need for
optional then.

The case where you should either use Optional or annotate something as
Nullable is the grey zone where this discussion is even possible. And there
we are talking mostly about things coming in from database calls and JSON
API's. Though other public API's work as well.

Take as an example JPA. If you create an interface extending JpaRepository,
it gets you a findById() method that returns an Optional to specify
there may not be a result from the corresponding SELECT running in the
backing database. In that case returning an Optional from a method is
reasonable. This is a public API, the caller is potentially Joe random
programmer, and annotations may not be available. The only other
alternatives are exceptions (problematic from performance viewpoint) and
just returning null and hoping for the best.
Clearly the latter approach wasn't good enough, since this method changed
to return an Optional some time ago.

Another example is fields in a DTO representing a JSON object. When you add
a new field and want to be backwards compatible in something like an update
endpoint you do not have a lot of options. Nullable annotated field is one,
Optional field is another. The first is enforced by linting tools and your
IDE, the other by the compiler. The first is slightly faster code with a
lower memory footprint, the latter forces correct behavior at compile time.

This is a trade-off, but again, using Optional is not an insane choice to
make, despite the opposition making it sound like it is. But it's not. And
I have a ton of working backwards compatible production code to prove it.

This is the whole point I am getting at. Sure, there are other ways to deal
with fields or objects that by design may be empty, the fact is that in
such cases people may reasonably choose to use Optional to make the intent
clear. And like it or not, such code is here today and doesn't look like
it's going anywhere.

On Fri, Jan 17, 2020, 03:58 Reinier Zwitserloot notifications@github.com
wrote:

How else do you protect against null pointer exceptions? The same way you
protect against any other bug. Tests, and writing better. Note that NPEs,
if they occur, mean there is some sort of misunderstanding. Either:

  • The API itself was unclear about what it returns, and some API user
    as a consequence did not realize it could return null.
  • The API was crystal clear, but the API user didn't read the docs or
    otherwise didn't follow instructions.
  • The API was crystal clear, and the user understood, but the user was
    under the mistaken impression that whilst the API was specced to perhaps
    return null, the user didn't think that could happen.
  • A bug in the code: The API says it can't return null, but it does
    anyway.

There are no other options.

Let's go through em:

  • API unclear: Well, fix that. With an annotation, such as @NonNull.
    This is backwards compatible (you can take existing API, such as for
    example j.u.Map's .get() method, and annotate it. You cannot replace
    its return value with Optional, which is why optional is bad and
    should not be used for this. It's inferior.
  • API clear, user didn't read: Same answer. An annotation will solve
    the problem. If the user is unwilling to look at that annotation and
    unwilling to use their tooling's support for it, that's on the user, no
    longer on the API designer.
  • API clear, user clear, but user has a mental model error: Optional
    is worse than NPE, by a considerable margin: If you use
    optional.ifPresent to map the value (because then the bug would appear
    as: The code silently does nothing. Even if you use the more appropriate
    optional.get(), then your code is just.. longer. in normal java world,
    you get an NPE, pointing right at your mistake. That's better than any
    other alternative.
  • API clear, user clear, API is bugged: A bug is a bug. Optional does
    not help here. It is possible the bug could have been avoided by having the
    API author be more careful with their no-value handling, which is a
    recursive application of this list.

There is no upside to using Optional in your returns over an annotation.

โ€”
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2264?email_source=notifications&email_token=AABIERILQXS4J3AQQISVW4DQ6ENERA5CNFSM4JCI5YR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJGI2NY#issuecomment-575442231,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERLVWE2EPR6QA7YJP43Q6ENERANCNFSM4JCI5YRQ
.

And there we are talking mostly about things coming in from database calls and JSON
API's. Though other public API's work as well.

We're _very_ far away from any sentiment about how NPEs are the biggest problem in javaland now, by zooming in to such a specific limited domain.

The only other alternatives are exceptions (problematic from performance viewpoint)

So your theory is that Optional wins on performance? That argument does not hold water. However, be that as it may, sure, conveying 'no results' via exceptions is probably not great.

However, you're so off the mark here. the only other alternative.

This. This is why I dislike it so much and why lombok will never, on my watch, get this feature.

I don't know if you're hyperbolising, in which case my complaint is that fans of Optional are being massively disingenuous and echochambering their way into preposterous statements, or, you're so tunnel visioned on optional that you can't think straight. In which case, evidently, optional is harmful to the brain.

Of course there are alternatives! Just look at j.u.map! Make findById(int id, T defaultValue), and then keep findById(int id) around which DOES throw. You don't need annotations, and this codes well regardless of circumstance. There are about 4 things you normally do in no-value cases:

  • Proceed as normal, with some default value.
  • Don't do anything at all.
  • Abort with error.
  • 'fork' โ€“ run a completely different bit of code.

Optional doesn't win over plain null in any of these, IF you set up your API right, such as above. Case 1 is catered to better with an explicit method, such as map's getOrDefault, case 2 is a specialization of case 4, and an if statement is perfectly fine. Optional's best effort for case 2 is ifPresent(lambda), which is just as much typing.

Abort with error is something null land gives you for free.

forking is, again, an if, and optional doesn't make this any easier either.

No, optional's only real upside is that it makes clear that no-value is an option, at the cost of a crap API. This is an unneeded sacrifice; annotations and proper documentation can do this job just as well.

Optional gets worse though; the above is why optional is bad in any language. But it is doubly bad in java.

We have TONS of existing libraries out there which use this returns null in case of no value as a principle. These __CANNOT__ be changed to optional without completely breaking backwards compatibility in a very drastic fashion.

So, pick an option:

  • We burn down all existing APIs, release v2's of everything. In case you're tempted, scala (the community) tends to do it this way, and, boy, it aint great.
  • We live with a mixed world, where about half of the APIs return optionals, the other half annotated nulls (because unlike optional, existing APIs can grow nullity annotations without breaking).
  • We get off this crazy optional train and forget about it.

This list is clearly exhaustive. And option 3 is clearly vastly superior over the other 2. Ergo, optional? No. QED.

On Sat, Jan 18, 2020, 02:38 Reinier Zwitserloot notifications@github.com
wrote:

And there we are talking mostly about things coming in from database calls
and JSON
API's. Though other public API's work as well.

We're very far away from any sentiment about how NPEs are the biggest
problem in javaland now, by zooming in to such a specific limited domain.

Possibly. I may be guilty of hyperbole. But then, so are you, calling this
"obviously insane".

The only other alternatives are exceptions (problematic from performance

viewpoint)

So your theory is that Optional wins on performance? That argument does
not hold water. However, be that as it may, sure, conveying 'no results'
via exceptions is probably not great.

It does:
http://java-performance.info/throwing-an-exception-in-java-is-very-slow/
We can have long discussions about that but there are numbers to back this
up. Exceptions really weren't designed for this, full stop.

However, you're so off the mark here. the only other alternative.
>

This. This is why I dislike it so much and why lombok will never, on my
watch, get this feature.

How so? I listed the options available. Did I miss anything?
This is not religion, I am dead serious. If there is a better way,
enlighten me. Don't throw an angry fit, it doesn't help.
And note: I am not arguing for this feature here. I think this is a
compromise between the "pro optional" and "against optional" camps that
satisfies neither.

Also, we should talk about requirements. Mine are:

  • We need it to communicate "This field or return value can be absent".
  • We need the callers to be made aware of that in a way they cannot ignore.
  • We cannot use checked exceptions for this, for several reasons, once of
    which is performance.
  • Must be understood by serialisation frameworks upon both serialisation
    and deserialisation of fields.
  • Must show up in generated documentation such as automatic swagger output.

Given the above list of requirements, I do not have a terrible amount of
options. It's annotate, or use the typing system. Exceptions already fail
it, since throwing an exception while (de)serializing means not
(de)serializing the object.

I don't know if you're hyperbolising, in which case my complaint is that
fans of Optional are being massively disingenuous and echochambering their
way into preposterous statements, or, you're so tunnel visioned on optional
that you can't think straight. In which case, evidently, optional is
harmful to the brain.

I am going to ignore this. If you want a reasonable discussion we can't get
emotional about it.
But please be aware of the requirements listed above.

Of course there are alternatives! Just look at j.u.map! Make findById(int

id, T defaultValue), and then keep findById(int id) around which DOES
throw.

See "exceptions", above. Filling in stacktraces is slow. We should not
resort to that unless there really is no other choice. And it will fail the
serialization / deserialization requirement.

You don't need annotations, and this codes well regardless of circumstance.

There are about 4 things you normally do in no-value cases:

  • Proceed as normal, with some default value.

In my case, Optional.empty(). There are models where "no value is an
option" is exactly what I want the model to communicate.

  • Don't do anything at all.

Needs a null check to be implemented. Deserialization will want to not
generate the field, which means null works just as well as Optional.
Serialization will fill the field with null, which will need to be
communicated through an annotation if you don't use Optional.

  • Abort with error.

Needs a null check or Optional.orElseThrow(). The latter allows us to chain
transformation steps through .map() and handle all failure to provide a
result in each of the transformations the same way, something null checks
do not provide.

  • 'fork' โ€“ run a completely different bit of code.

Needs a null check. We get this for free if we return an Optional, since
we don't need to hand-code an equivalent to .isPresent() ourselves.

>
>

Optional doesn't win over plain null in any of these, IF you set up your
API right, such as above.

How do you define "winning", exactly?

Case 1 is catered to better with an explicit method, such as map's

getOrDefault,

Not if we are talking about a public JSON model where that method cannot be
exposed to the consumer of the model.

Apart from that, sure, getOrDefault() can be a way to do this. If the
"default" can be a lambda function we can even run custom code. But it
doesn't help for everything. Especially if you need to pass that value on
to other layers of the code.

And even then, we'd have to implement getOrDefault() somehow. As far as I
know, lombok doesn't help us with that, so that means adding boilerplate
code to classes which we wouldn't need with Optional.
Plus, this thing is non-standard, so Swagger and documentation frameworks
like that won't pick this up.

As an aside though - how would you feel about a Lombok annotation that
creates this "getOrDefault()" method?

case 2 is a specialization of case 4, and an if statement is perfectly

fine.

It is.
My argument isn't that null checks aren't a way to solve it, my argument is
that Optional is a valid way to solve this as well.
But the real question remains: how do you communicate that that null check
is necessary in the first place? If a method needs to change and suddenly
can return "no value", how do you ensure the called all change as well?
Rename it? Provide a "getOrDefault()" version? Is it truly unreasonable to
simply return an Optional, or is it just one way to deal with it that has
it's own tradeoffs?

Remember, you were saying it is "clearly insane", when I contend that it is
not unreasonable. Stream.findAny() returning it is reasonable, why isn't it
reasonable for JPA's findById() or something like car.getCruiseControl() ?

Optional's best effort for case 2 is ifPresent(lambda), which is just as

much typing.

On the consumer side sure, but on the producer side you'd have to write
that method, and something like

@NotNull @Builder.Default private final Optional<CruiseControl> cruiseControl = Optional.empty();

clearly wins in terms of lines of code required there.

Abort with error is something null land gives you for free.
>

But usually it's not an error.

forking is, again, an if, and optional doesn't make this any easier either.

It makes it easier to provide that variant and it makes it easier to
communicate the intent. So I disagree.

No, optional's only real upside is that it makes clear that no-value is an
option, at the cost of a crap API. This is an unneeded sacrifice;
annotations and proper documentation can do this job just as well.

That this is a "crap API" is clearly your opinion.
I disagree, and given the requirements listed above, I think I can back
that up.
Proper documentation can't fail the compilation if someone forgets the
check. Annotations only work if your toolchain is set up for it and works
properly. It does not provide guarantees that people won't forget to
check for the absense of the value, which Optional does.

Optional gets worse though; the above is why optional is bad in any

language. But it is doubly bad in java.

Then why are there languages like Haskell providing their variant of
Optional (
http://hackage.haskell.org/package/base-4.12.0.0/docs/Data-Maybe.html) as
the only way to deal with this kind of usage?

And why is it bad in Java at all?

I contest that 20 years of ingrained "we must check for null, and
returning null is the only way" is a cultural thing. The fact that using
Optional for fields and return values is causing certain engineers to react
in such a visceral and hostile way does not mean it's actually evil.

We have TONS of existing libraries out there which use this returns null
in case of no value as a principle. These CANNOT be changed to optional
without completely breaking backwards compatibility in a very drastic
fashion.

And yet that is exactly what JPA did. And the world did not go down in
flames. People fixed their builds, and moved on.

So, pick an option:

  • We burn down all existing APIs, release v2's of everything. In case
    you're tempted, scala (the community) tends to do it this way, and, boy, it
    aint great.

Who says we have to do anything of the sort? Existing libraries can keep
doing what they are doing as far as I am concerned. I am only talking about
my own codebase, and how I protect myself against unexpected nullpointers
in code I control.

Don't forget, changing the return type of a method, or the types of fields
in a class, will simply break the compilation unit. It won't unexpectedly
and silently break things at runtime. This will annoy a few engineers
during a library upgrade for all of five minutes and then they will simply
handle it.

>

  • We live with a mixed world, where about half of the APIs return
    optionals, the other half annotated nulls (because unlike optional,
    existing APIs can grow nullity annotations without breaking).

Sure. And annotated nulls is fine. It's a choice you can make, just like
Optional is.

>

  • We get off this crazy optional train and forget about it.

Not going to happen. Cat is out of the bag, Optional exists. Not all of the
language designers or java engineers out there may be happy about it, but
that's change for you.

Meanwhile, this needs adressing in the real world:
https://blog.overops.com/the-top-10-exceptions-types-in-production-java-applications-based-on-1b-events/

And you can try and handwave it away, but NullPointerException is real,
too, and I as an engineer need to find ways to protect my code against it.

This list is clearly exhaustive. And option 3 is clearly vastly superior
over the other 2. Ergo, optional? No. QED.

That is your opinion only. I contest using that class is a valid Option,
if you will forgive my pun.

โ€”
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2264?email_source=notifications&email_token=AABIEROLDUCFYKY2QEBCLVLQ6JMSHA5CNFSM4JCI5YR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJJNIXI#issuecomment-575853661,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERK2Z4N366N4SW5DLVTQ6JMSHANCNFSM4JCI5YRQ
.

If there is a better way, enlighten me. Don't throw an angry fit, it doesn't help.

I spent quite some time to show you the optional-free alternative, and you think that was an angry fit? You must have misunderstood. It feels like you typed this reply 'as you read it'. That's usually not a great way to hold a conversation on forums and bugtrackers, in my experience. Perhaps this is a bit of a projection and you are a bit annoyed that your considerable investment in updating your codebase to use Optional is being questioned, and frustrated that Lombok is not likely going to get anything to help you eliminate some boilerplate. This is understandable.

Meanwhile, this needs adressing in the real world: [link to that tired old bit about NPEs being common]

I already explained to you that this is a problem of API designers being unclear about how their APIs work, and it is not useful to shove the blame in the shoes of nullity annotations, which solve the problem just as well.

It is clear you are extremely set in your ways and perhaps so am I, because now we're just going round in circles.

Apparently, then, this isn't a debate; all we're doing is shouting at unmovable objects. Every point you raised in favour of Optional seems either false to me, or otherwise not in any useful way superior to nullity annotations, but further debate evidently does not seem fruitful.

The final word is: nullity annotations cover every single last precondition you wrote, AND it covers the ones I have (avoiding splitting libraries in twain as much as possible, primarly) which Optional does not, and it seems like there's no further point trying to explain to you why I feel like it works that way.

All that is left to me is to give you clarity: Lombok will not support optional getters. Period.

I feel the odds further discussion leads to good things for Project Lombok has dropped too low to continue to spend time on this. I suggest that you don't waste any further time on it either.

I agree, actually.

Two other things I agree with you on:

  • I do not really want Optional getters either.
  • The library thing is at least not an unreasonable argument.

I will leave it at that, maybe we can have a beer over it some day and
laugh. It's not like this whole discussion is actually important :p

On Sat, Jan 18, 2020, 22:51 Reinier Zwitserloot notifications@github.com
wrote:

If there is a better way, enlighten me. Don't throw an angry fit, it
doesn't help.

I spent quite some time to show you the optional-free alternative, and you
think that was an angry fit? You must have misunderstood. It feels like you
typed this reply 'as you read it'. That's usually not a great way to hold a
conversation on forums and bugtrackers, in my experience. Perhaps this is a
bit of a projection and you are a bit annoyed that your considerable
investment in updating your codebase to use Optional is being questioned,
and frustrated that Lombok is not likely going to get anything to help you
eliminate some boilerplate. This is understandable.

Meanwhile, this needs adressing in the real world: [link to that tired old
bit about NPEs being common]

I already explained to you that this is a problem of API designers being
unclear about how their APIs work, and it is not useful to shove the blame
in the shoes of nullity annotations, which solve the problem just as well.

It is clear you are extremely set in your ways and perhaps so am I,
because now we're just going round in circles.

In which case, if debate is not on the table, and all we're doing is
shouting at unmovable objects. Every point you raised in favour of Optional
seems either false to me, or otherwise not in any useful way superior to
nullity annotations, but further debate evidently does not seem fruitful.

The final word is: nullity annotations cover every single last
precondition you wrote, AND it covers the ones I have (avoiding splitting
libraries in twain as much as possible, primarly) which Optional does not,
and it seems like there's no further point trying to explain to you why I
feel like it works that way.

All that is left to me is to give you clarity: Lombok will not support
optional getters. Period.

I feel the odds further discussion leads to good things for Project Lombok
has dropped too low to continue to spend time on this. I suggest that you
don't waste any further time on it either.

โ€”
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2264?email_source=notifications&email_token=AABIERLVQJLQ2WE7NUOGAEDQ6N2U7A5CNFSM4JCI5YR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJKCXAQ#issuecomment-575941506,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERKIW6IYRG4JXDQVRW3Q6N2U7ANCNFSM4JCI5YRQ
.

Just my two cents here:

I don't care much for serialization either -- however, I agree with Goetz' guideline for using Optional as a return type, not a field type, because it just makes a lot of sense. When you're writing a class, say Person, the fields of that class (name, age, etc) are best expressed as their respective types, which keeps things clutter-free and simple.

Optional was introduced to, as Goetz puts it, force the caller of a method to actively unwrap its returned object and consider the possibility of no value. Which is why people are using it when designing APIs, because it enforces this contract at the code-level and not with an annotation, which compilers/IDEs may or may not notify the developer of. So while null is just fine for internal implementation, Optional is very helpful for public APIs. "The API itself was unclear about what it returns" can be addressed with javadoc/annotations, but is _better addressed_ with this explicit type.

I noticed in your 'four cases' where you attack the utility of the Optional type that you discount the utility provided by the class' methods, like map, flatMap, and a few others (I too am not a fan of ifPresent, admittedly).

But forget all of this and just consider the following:

While this is your library and you can do whatever, your personal opinion that "it's laughably insane to want it. Optional is for potentially-no-value returning terminals on stream ops. AND NOTHING ELSE." is a bizarre reason why this shouldn't be supported by Lombok -- this opinion of yours is hardly a consensus, not even close. At the very least @Getter.Optional could be an experimental feature to get community feedback. Or if not that, then just tabling it as a possible feature to consider for the future. Not saying your (slightly eccentric) opinion is wrong, but I'm questioning why it and it alone should pose an obstacle to at least seeing how the broader community feels about this.

Was this page helpful?
0 / 5 - 0 ratings