Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow noIsPrefix for getters on a per-getter basis #2464

Closed
hakanai opened this issue May 15, 2020 · 12 comments
Closed

Allow noIsPrefix for getters on a per-getter basis #2464

hakanai opened this issue May 15, 2020 · 12 comments

Comments

@hakanai
Copy link

hakanai commented May 15, 2020

Describe the feature

Depending on the name of the property, sometimes isUpdated() makes sense and other times isWasUpdated() makes no sense so naming it getWasUpdated() is preferred.

We can configure all such methods to be named with get in the config file of course, but sometimes I'd still like to be able to have methods named with is. :(

So I'd like something along the lines of:

    // generates getter called isAccessible()
    @Getter
    private boolean accessible;

    // generates getter called getWasUpdated()
    @Getter(noIsPrefix = true)
    private boolean wasUpdated;

Describe the target audience

People who are opinionated about the naming of bean methods. ;)

Additional context

None?

@Maaartinus
Copy link
Contributor

IMHO there are just three usable options:

  • You have to be javabeans compatible. Then you're partially out of luck as the crap is incompatible with itself (for fields named like uShaped), but in general, it works out of the box.
  • You want to be consequent and always use "get" and "set" prefix. Then put lombok.getter.noIsPrefix = true in lombok.config.
  • You want to use the fields name as for the getter and setter (like in a builder). Then use lombok.accessors.fluent = true.

Of course, every rule has exceptions. The exceptions should be rare, otherwise the users of the library get lost guessing. Lombok doesn't support exceptional cases, it only cares about boilerplate. So I guess, you need to write some getters manually (possibly with @Getter(AccessLevel.NONE)).

Disclaimer: I'm not a project maintainer.

@hakanai
Copy link
Author

hakanai commented May 17, 2020

JavaBeans actually permits both is and get for boolean properties, so I don't know what you're talking about. It has always been about whether reading the method name as "is something" made sense or not, for whether you named it as is or get.

@MichelMunoz
Copy link

MichelMunoz commented May 29, 2020

JavaBeans actually permits both is and get for boolean properties, so I don't know what you're talking about. It has always been about whether reading the method name as "is something" made sense or not, for whether you named it as is or get.

Nobody said the contrary. There must be a misunderstanding, @Maaartinus gave you rationales and solutions to your problem

  • your objects api should be consistent (coherent, "guessable", easily discoverable...), thus the general configuration solutions provided by @Maaartinus + the defaut behavior of Lombok (boolean -> is, Boolean -> get) allow that
  • but, there are always rare exceptions to the general consistency, in which case, as indicated by @Maaartinus , Lombok already provides you with two solutions (implementing the getter or changing from boolean to Boolean) as illustrated here:
@Getter
class SomeClass {   
    private boolean value1;
    
    private boolean value2;
    
    private Boolean value3;
    
    public boolean getValue2() {
        return value2;
    }
}

will give you a isValue1(), getValue2(), getValue3().

@hakanai
Copy link
Author

hakanai commented May 30, 2020

Whether the "exceptions" are rare or not depend on what you're writing. In our case it's heading towards 50/50, and I can easily imagine cases where it might be the majority.

I know that I can just implement the method. I can avoid Lombok entirely, with the same end result. I was just asking for a new feature to be able to configure this on a per-property basis, so that I can remove all trivial getters instead of just half of them.

@randakar
Copy link

randakar commented May 30, 2020 via email

@hakanai
Copy link
Author

hakanai commented Jun 1, 2020

The codebase is just several gigabytes in size, so even a proportionally small number of boolean getters adds up in numbers overall. But the real answer is probably "because all these dumb JSON serialisation libraries force us to have them". :(

@randakar
Copy link

randakar commented Jun 1, 2020 via email

@hakanai
Copy link
Author

hakanai commented Jun 1, 2020

They're not Boolean, just boolean.

@randakar
Copy link

randakar commented Jun 1, 2020 via email

@rzwitserloot
Copy link
Collaborator

We'd be adding an annoparam that everybody sees and which is completely irrelevant for 90%+ of all properties (all non-booleans), just to cater to a use case where you're replacing a one-liner method with an annotation that contains as many characters as the very code it is supposed to replace.

Not worth the maintenance burden and throwing this in everyone's face, by miles.

@hakanai
Copy link
Author

hakanai commented Jan 3, 2021

It's only a one-line method until you open your eyes and see the rest of the required boilerplate. So you'd actually be replacing 4 or 5 lines with a single parameter, which is definitely worth it for the readability if not the time saved. And people who don't want to use it can just ignore it. They don't have to see it in their code unless they're using it.

@rzwitserloot
Copy link
Collaborator

rzwitserloot commented Jan 3, 2021

you open your eyes

Crap on ideas all you want. But don't make it personal, please. Tone down your comments, this isn't the first time you've come across as abrasive (and that's coming from me, which presumably says something). The point is to be constructive, insinuating that other participants in this issue are lazy or incapable of drawing out ideas to their logical conclusion is decidedly not constructive.

And people who don't want to use it can just ignore it.

IDE autocomplete dialogs will show it. This line of arguing is not going to change the status of this issue.

and see the rest of the required boilerplate

Exactly. Which is why this feature is dead in the water. The maintenance burden is significant, and that the linkage is weird: Changing the getter name doesn't just change the getter lombok generates, but also the equals and hashcode, with/builder 'deconstructors', and who knows what else.

The right place for messing with names is @Accessors. Which was never intended to mainly be used on a specific field, so it's not a light feature request if that is to carry the burden of picking the entire accessor name for a single field. However, @Accessors's technical implementation in lombok already 'cascades': Check the field, and if it is not there, check the class, and if that's not there, check the config. That should mesh well with having it carry the job of what you're trying to accomplish with this feature request.

To be crystal clear: This is never going to be added to @Getter unless it is a complete reimagining of what @Accessors currently does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants