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

@Getter custom method name #836

Closed
lombokissues opened this issue Jul 14, 2015 · 25 comments
Closed

@Getter custom method name #836

lombokissues opened this issue Jul 14, 2015 · 25 comments

Comments

@lombokissues
Copy link

Migrated from Google Code (issue 801)

@lombokissues
Copy link
Author

👤 mibac138   🕗 Mar 21, 2015 at 13:35 UTC

I think it would be great if we could in @ Getter pass argument which will be methods name. For example: @ Getter(name="isTrue") or @ Getter(name="isRunning")

@lombokissues
Copy link
Author

👤 mibac138   🕗 Mar 21, 2015 at 13:36 UTC

For @ Setter it would be good too I think.

@lombokissues
Copy link
Author

End of migration

@mibac138
Copy link

Any progress on this?

@dslivka
Copy link

dslivka commented Aug 12, 2016

Any progress on this? One motivation would be customization of getter/setter names for field names starting with underscore. In my case, I need setter named setIF() for field protected String _if;. I do not want to rename the field (it was generated from XSD and Dozer relies on the name for mapping). CXF/JAXB generates seter setIF(), also Dozer seems to be relying on such getter/setter name. Customization of getter/setter name in Lombok would be the most elegant and easy-to-read solution.

@ivankarpey
Copy link

Yep, will be nice to have this imlemented

@rspilker
Copy link
Collaborator

Regarding underscores: you can add them as field prefix in lombok.config:

lombok.accessors.prefix += _

The would remove them from the getter/setter names,

@dslivka
Copy link

dslivka commented Sep 27, 2016

Nice try, however, it's getting even worse: "Not generating getter/setter for this field: It does not fit your @accessors prefix list".
2016-09-27 17_58_57-spring - jrfo_src_main_java_com_ibm_sk_jrfo_ws_nice_dto_zs_trvervxo java - sprin

@blindgoat
Copy link

blindgoat commented Feb 14, 2017

I too would like to be able to set custom names or characters to ignore. In my example, I would like getter/setter names to ignore the _ on variables.

I'd like private String _username; to generate getUserName(), not get_userName()

@MedicOP
Copy link

MedicOP commented Feb 26, 2017

Would be a nice feature

@konoplev
Copy link

konoplev commented Mar 3, 2017

@dslivka @blindgoat please take a look at @Accessors annotation. It has two targets: type and field. So you can use it for a specific field. Just add @Accessors(prefix = {"_"}) to your _id / _username field.

@o-pikozh
Copy link

o-pikozh commented Aug 16, 2017

Another possible use-case

Boolean properties don't always follow the same scheme — even within the same class. For example:

  • It is ok to have isEnabled and setEnabled methods for boolean property enabled of class Control; or isVisible and setVisible methods for boolean property visible of class Control.
  • But it is not ok to have isAdministrator and setAdministrator methods for property isAdministrator of class Person; or isRoot and setRoot for property isRoot of class Node.
    Method names setAdministrator and setRoot suggest that a parameter is reference to another object (e.g. setAdministrator(Person administrator), setRoot(Node root)), not boolean.
    As for me, it is reasonable to retain full method names (getIsXxx and setIsXxx) even for boolean properties when xxx is not an adjective-or-participle, but is a noun (so that method name setXxx would wrongly suggest that it sets reference to another object, while it actually sets boolean flag).

And the problem is hardened by the fact that both schemes could be used within the same project and even within the same class (even the same class Node may have boolean isRoot (getIsRoot, setIsRoot), Node root (getRoot, setRoot) and boolean enabled (isEnabled, setEnabled)).

P.S.: Another possible solution for the problem described in this comment might be not to remove is from field names. I.e. if field name is visible → let it be isVisible/setVisible (unless lombok.getter.noIsPrefix is set to true, of course), if field name is isAdministrator → let it be getIsAdministrator/setIsAdministrator (not isAdministrator/setAdministrator; regardless to the lombok.getter.noIsPrefix setting). But, I suppose, it would be too incompatible change.

Resume

In fact, current getter/setter name generation scheme is non-ideal. And whatever we will change it to, it would be non-ideal either. Allowing custom getter/setter names would make lives of those-who-struggle-from-current-glitches (e.g. from #757) easier.

@rzwitserloot
Copy link
Collaborator

I feel this issue is appropriately addressed in lombok right now with @Accessors.

@o-pikozh
Copy link

o-pikozh commented Apr 10, 2018

@rzwitserloot, surely not.

  1. @Accessors sets style per-class. But, as I described above, one class may use different schemes. Sorry, I was wrong in this point; never-the-less the two other points are true.

  2. @Accessors can specify what prefixes to remove. But what is the most awaited (at least by me) is a possibility to keep prefix in certain cases.

  3. @Accessors is limited (non-flexible).

@JoshMcCullough
Copy link

JoshMcCullough commented Jun 11, 2019

Sometimes is is just wrong for booleans. E.g. private boolean continueExecution would become isContinueExecution() when in reality I'd want it to be shouldContinueExecution(). I can work around this by implementing the getter/setter myself, but I'd much rather do @Getter(prefix = "should") or similar on the field.

Please correct me if this is already possible on a per-field basis.

@Maaartinus
Copy link
Contributor

@JoshMcCullough It's not possible and there are IMHO only three sane choices for a getter name (all three are supported):

  • If you have to follow the Javabeans crapification, use "is".
  • If you want to have a machine-friendly system, use "get".
  • If you want to have short names, use no prefix.

Using anything else adds some semantics to where it IMHO does not belong to. A variable called "continueExecution" should be always called just that (possible with a semantic-less prefix), not "shouldContinueExecution" nor "mustContinueExecution" nor "willContinueExecution". IMHO applying grammar or making it sound better is much less important than consistency - one day, you may need to make some tool understand your conventions. Sometimes it leads to misleading names, like "getReady", but then I'd just call my variable "isReady" and the name of the getter "getIsReady" can't mean anything but "get isReady". YMMV.

That's just my personal opinion. I'm not in charge of the project, but I guess it won't get implemented because of these reasons.

@JoshMcCullough
Copy link

@Maaartinus Fair point, in this example continueExecution is fine as the field and getter, with a reasonably named setContinueExecution to go with it.

@o-pikozh
Copy link

o-pikozh commented Jun 18, 2019

@Maaartinus, you somehow miss very important thing: if you call you variable isReady, its getter and setter would be (in Lombok 1.18.8) isReady and setReady, not getIsReady and setIsReady. There is no straightforward way for you to create getIsReady and setIsReady methods. That's what all this thread is about.

@Maaartinus
Copy link
Contributor

@o-pikozh It's you who is missing thing:

https://projectlombok.org/features/GetterSetter

lombok.getter.noIsPrefix = [true | false] (default: false)
If set to true, getters generated for boolean fields will use the get prefix instead of the defaultis prefix, and any generated code that calls getters, such as @ToString, will also use get instead of is

It's somewhat hidden... and probably not much used. IIRC it was me, who added this feature (but I'm not sure anymore) as I hate the default.

@o-pikozh
Copy link

o-pikozh commented Jun 18, 2019

@Maaartinus, AFAIK, in that case it would be getReady and setReady, but not getIsReady and setIsReady.

(And I am not even talking about the fact that it's global setting, not field-wide and not even class-wide.)

@o-pikozh It's you who is missing thing:

https://projectlombok.org/features/GetterSetter

lombok.getter.noIsPrefix = [true | false] (default: false)
If set to true, getters generated for boolean fields will use the get prefix instead of the defaultis prefix, and any generated code that calls getters, such as @ToString, will also use get instead of is

It's somewhat hidden... and probably not much used. IIRC it was me, who added this feature (but I'm not sure anymore) as I hate the default.

@Maaartinus
Copy link
Contributor

@o-pikozh AFAIK, it must be getIsReady and setIsReady, at least that's how I always wanted it and how I (believe I) implemented it. Anything else would be a nonsense - either you want to stick with the crapification or you want some system.

I've just checked it and it works like I wrote. I had to check as I'm using

lombok.accessors.fluent=true
lombok.accessors.chain=true

which makes it irrelevant.

And I am not even talking about the fact that it's global setting, not field-wide and not even class-wide.

Do you want talking it? I doubt it, as the only IMHO plausible reason for this setting is that you want a systematical naming. This makes per-field or per class settings useless as they'd spread even more chaos than the crapification.

@o-pikozh
Copy link

o-pikozh commented Jun 19, 2019

@Maaartinus, you're right, it's getIsReady and setIsReady in lombok.getter.noIsPrefix = true mode. At least, documentation then is misleading, because it states "will use the get prefix instead of the default is prefix" (and says nothing about fixing the prefix of setter).

The lombok.getter.noIsPrefix flag really makes lombok more usable, though it still looks somewhat counter-intuitive for me.

@Maaartinus
Copy link
Contributor

@o-pikozh Agreed that the name isn't very good (I originally proposed consequentBoolean, but this wasn't good either), but this train has departed long ago.

However, anyone can improve the documentation, just clone the repo, edit website/templates/features/GetterSetter.html and submit a PR. I guess, the isReady example would be useful, but I'm afraid that more than a single sentence won't be welcome (other features don't get long explanations either and it's pretty long already). Note that I'm just guessing; I'm not in charge of the project.

@o-pikozh
Copy link

o-pikozh commented Jun 19, 2019

@Maaartinus, anyone can improve the code as well, the same as documentation (but that doesn't mean we can't discuss current state).

What bothers me the most in the current default behavior is not what exactly prefix is added, but the fact that existing prefixes are removed. I.e. I don't care too much whether enabled field creates getEnabled/setEnabled methods or isEnabled/setEnabled methods, but I expect isEnabled field to create getIsEnabled/setIsEnabled methods (or at least isIsEnabled/setIsEnabled methods) -- not isEnabled/setEnabled. I.e. I dislike the fact that, by default, is prefix is removed (then, of course, it's re-added in getter, but setter gets just set), and for years I was looking for a flag that will allow lombok not to remove is. And, of course, I seen lombok.getter.noIsPrefix, but from documentation it looked that it would just turn isEnabled into the getEnabled and nothing more (thus just changing a weird behavior into even more weird one), so I never tried it. What documentation, IMHO, should say about lombok.getter.noIsPrefix in the first order is that it prevents removing is prefix from the field name (so isEnabled field creates xxxIsEnabled/yyyIsEnabled methods, not xxxEnabled/yyyEnabled), and only in the second order that it changes the prefix adder to boolean field getters from is to get.

@Maaartinus
Copy link
Contributor

anyone can improve the code as well

@o-pikozh Sure, but just changing the name won't be any improvement, even if the new name was perfect (because of compatibility). I proposed you (and everyone else) to improve the docs as a starting point. I fully agree about what you wrote about the brain-damaged prefix removal; that's one reason why I say "javabeans crapification". I also agree that "noIsPrefix" says nothing about the removal prevention. As changing the feature name improbably gets approved, I'd suggest you to PR the docs.

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