projectlombok / lombok

Very spicy additions to the Java programming language.
https://projectlombok.org/
Other
12.86k stars 2.38k forks source link

Feature request: Add option to @Getter to allow Optional returns for nullable fields #1957

Closed michaelboyles closed 3 years ago

michaelboyles commented 5 years ago

I use Lombok's @Getter whenever possible. Almost all of the pure getters I find myself writing these days are for java.util.Optionals.

private final String foo;

public Optional<String> getFoo()
{
    return Optional.ofNullable(foo);
}

Changing the field declaration to Optional<String> is not a great solution because any setters would expect an Optional. It's also frowned upon to use Optional as a field value (IntelliJ will give you a warning, for example).

It would be nice if @Getter had an option which allowed me to generate this automatically. For example:

@Getter(optional = true) private final String foo;

This would obviously default to false.

rafaelcoelho commented 3 years ago

Come on @rzwitserloot, Optional is a officially Java type and Lombok is an extremely useful library to wrap-up boilerplate coding, that's it! Thus, please don't take the community wrong (i.e. the audience in this thread) for requesting the Lombok supports for Optional and will be up to the end user to decide how they want to use it. How about to push it as experimental and see the feedback from the community?

We super respect your view and arguments but the Optional type is part of JDK a long time ago and looks like Lombok is not in charge to advocating how it should be adopt or not!

Guys lets take it easy!

2is10 commented 3 years ago

@rzwitserloot Thank you for this very useful library! And thank you for sharing your position on null and Optional. I highly value informed opinions backed by thoughtful, formal analyses like yours.

I have a small request, from one Lombok contributor šŸ˜‰ to another. Please reflect and consider for a moment: Is there anything else as important to you as the eventual fate of Optional? What about the liberty and personal choice that we all enjoy to create what we wantā€”the very liberty that enabled you and @rspilker to create Lombok and help millions of others create more efficiently?

Letā€™s review some facts:

  1. Optional was introduced to the language 7+ years ago.
  2. It hasnā€™t been deprecated yet.
  3. People are using it.
  4. Whether people use Optional is outside your control.
  5. A significant number of Lombok users are manually writing getters that return Optional.
  6. You have the power to make them more productive even as you continue the fight against Optional.

Weā€™re not asking you to perform an abortion or make a cake for a gay wedding. Weā€™re just asking you to please help us save a few keystrokes (and seconds reading boilerplate code) nearly everyday at work. šŸ˜„

If Optional gets deprecated someday, I promise not to be upset when you drop support for Optional getters. Iā€™llĀ applaudĀ yourĀ success!

ailjushkin commented 3 years ago

@2is10 maybe someone should be better to create a pr? you create a pr, everyone approve it, profit?

walec51 commented 3 years ago

@ailjushkin maybe create a fork your self and stop expecting some one else to do the work for you

or at least read the entire conversation - there is no point of doing a PR that will not be accepted

ailjushkin commented 3 years ago

@ailjushkin maybe create a fork your self and stop expecting some one else to do the work for you

or at least read the entire conversation - there is no point of doing a PR that will not be accepted

Okay.

valtsmazurs commented 2 years ago

While I agree that Optional fields would not be the appropriate use case, @Getter(optional = true) would be useful for nullable fields. I and at least few other developers that I personally know (I'm not aiming for "many people") would wrap the nullable values in Optional anyway just because we like the functional programming approach. This feature would reduce the boilerplate code which IMO is the main purpose of Lombok, isn't it?

rzwitserloot commented 2 years ago

@2is10:

What about the liberty and personal choice that we all enjoy to create what we want

Did you.. just... equate 'personal freedoms' in the broadest sense with "Please sign up to support something for a decade that you do not want"?

The very liberty that enabled you and @rspilker to create Lombok and help millions of others create more efficiently?

Us not adding this feature means that we are taking away your liberties?

This is incredibly inappropriate.

@ailjushkin:

everyone approve it, profit?

This is... not how open source projects work. I hope not - any project that does work that way, run for the hills. PRs need to be supported. FOSS projects aren't democracies, they are meritocracies, and you really, really wouldn't want it any other way.


That's enough nonsense for the day. We're dutch. Fawning commentary that then asks us to take on burdens just comes across as rude (as you aren't addressing any of the technical points that were raised, i.e. you're ignoring the details and just tossing noise in, hoping that saying nice things about lombok somehow makes us forget about all the myriad technical issues). I understand it wasn't intended as such, so I've taken no offense at all. Just trying to highlight that it's not going to work.

This feature remains closed, PRs will not be accepted.

Any further comments that add nothing technical will be deleted henceforth. Not as punishment or because feedback is not desired - but because, as the length of this thread shows, you're not adding anything useful to the discussion. Yeah, there's plenty of folk out there who think this would be a fantastic addition to lombok. We know. You don't need to keep saying it. The reasons this issue isn't being re-opened aren't out of spite or out of a lack of understanding about much people think they want this. We know.

Eboubaker commented 2 years ago

So the reason this can't be added is just a bias towards not liking the optional design huh.

topr commented 1 year ago

Gee, quite a heated debate on this one šŸ”„

I agree that using Optional as an method/constructor argument type or a field type is an anti-pattern.

However, having an automatically generated getters which returns Optional for a field annotated with @Nullable, that would be handy.

I often take advantage of either @Getter or @Value annotations to reduce boilerplate, but for the fields where a default value isn't available to resolve nullability, I end up with some boilerplate added back, example as follows:

@Value
class Foo {

    @NonNull
    String bar;

    @NonNull
    String baz;

    @Nullable
    @Getter(NONE)
    String quaz;

    public Optional<String> maybeQuaz() {
        return Optional.ofNullable(quaz);
    }
}

I appreciate there would be a getter generated with @Nullable for the quaz field, but compiler won't fail on its return value if there is an operation performed on it which inflicts NPE.

So from practical point of view, it's safer to suppress generation of the default getter, and let user of the added "maybe" getter to deal with the Optional return type, whilst nullability stays private and encapsulated (by the @Value annotation all fields become private final).

This enforces handling lack of the value on the use side, while receiving a null can go šŸ’„ unnoticed until runtime. Fail fast is better, isn't it?

I'll keep doing what I'm doing, but it is no easy to make a team follow a convention which require extra manual steps. Not because they disagree, they actually agree. Because it is in a human nature to omit what doesn't have to be done either by negligence, by forgetting it or by a mistake.

Lombok has been founded to reduce the boilerplate in the first place, wasn't it? Having such a "maybe" getter (of any name convention) generated automatically on the @Nullable fields, which helps to encapsulate and immobilize nulls, would be simply much practical and helpful in reducing boilerplate further.

Anyway, I'm grateful for this lib regardless of this matter. Many kudos to the authors and maintainers, cheers folks! šŸ™

sahendrickson commented 1 year ago

In Google, we use Optional. So, as a data point, it's a well established practice. The easy fix would be to add an option to @Getter so that backwards compatibility is not an issue. But, to say that you won't even consider a PR because of (let's admit) a personal preference that you want to force everyone to follow is unfortunate for the project, I think.

boosh commented 1 year ago

@sahendrickson it would be great if Google created and maintained the requested fork :-)

egorksv commented 10 months ago

That's enough nonsense for the day. We're dutch. Fawning commentary that then asks us to take on burdens just comes across as rude (as you aren't addressing any of the technical points that were raised, i.e. you're ignoring the details and just tossing noise in, hoping that saying nice things about lombok somehow makes us forget about all the myriad technical issues). I understand it wasn't intended as such, so I've taken no offense at all. Just trying to highlight that it's not going to work.

Thanks a lot for making this clear. I will make sure to de-lombok all our projects, as risks of relying on a FOSS project maintained by people with egos eclipsing that of Donald Trump's are just way too high to justify

brainbytes42 commented 8 months ago

Optional is a useful part of Java.

And yes, not supporting Optional getters takes away the developer's freedom of choice, if he wants to use Optional (without coding by hand every time). So @2is10's Remark is not inappropriate, but valid.

But I also see your point @rzwitserloot, that you think it's your free choice to not support it and see the effort to maintain this feature. This is valid as well, but unfortunate. I doubt the maintenance-effort will be that high and I would like to include your responsibility into the equation, as you'r maintaining a popular library and there is obviously a high demand for this single feature. (Wich is not backwards incompatible, if the default setting leaves everything as is.)

I don't assume any change in mind, but I do want to document another request for this feature.