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
Wish: makeFinal parameter for Getter/Setter annotations #1456
Comments
What would be the added value to final getters/setters over public members ? |
@grimly The obvious ones: 1. They (can't be overridden but they) still can override a method. 2. They are methods (my rule is: no non-private non-constant fields). |
@grimly, do you mean: over the public fields? Well:
|
I think you misunderstood my comment. I asked what would be the added value from final getter and setter over a public field (as I wrote member, my bad). If you mark your getter and setter final and they are as the dummy ones, they are no differents to public fields I personally forbid myself the use of final modifier from the countless errors I encountered with container environments (jee, spring), and overriding might be useful at times, so I can't see the use of it but since the discussion is open I want to know what would be your use of it. |
I answered exactly that question. Do I write clear enough in |
Hmmmm. Fine to me. I thought there was more. Nevertheless , to be able to generate final getters and setters do not alter current usages of lombok. If the cost is low, what do we lose ? |
Locked only for incompatible changes. E.g. you can add new method, but not turn method into field or field into method.
Sorry, I haven't got the question. What do we lose from what? |
|
I stumbled upon this one myself today. Had to refactor a non-Lombok project to use Lombok but it has simple getter/setter which are final. I can try and implement this myself if I can get some agreement that it would be useful/doable. I've browsed the source code for Lombok and I believe adding a |
No plans on supporting creating final getters/setters by Lombok? |
Can someone please explain why having a I'm looking for an actual use-case, not a "because I want to have sub-classes, but they are not allowed to override my getter/setter." |
@rspilker, sorry, maybe I ask something stupid, but why don't you consider "I want to have sub-classes, but they are not allowed to override my getter/setter" to be an actual use case? (What do you mean by actual use case then?) |
Can you describe a common, real world (non-hypothetical) scenario where the class in not final, but the getters and setter must be final, and that that's the best solution for the problem. In my experience, lots of code bases are favoring composition over inheritance. Most data classes don't have sub-classes. And if they do, there is no requirement that the getters/setters need to be final. I mean, that invariants are fundamentally broken. One could always argue that it help reasoning about the code in case you want to execute a refactoring. But that alone is not good enough to add additional parameters and complications to We're really interested. Maybe there is a good reason we don't know about. But so far, we haven't heard compelling arguments. The best argument so far was "It speeds up introducing Lombok in legacy code bases". And, although compelling, that alone not good enough. |
@rspilker, for me "I want to have sub-classes, but they are not allowed to override my getter/setter" is a real-world every-day (every day when I use Java) scenario.
Exactly. That's why I tend to have things But in some cases we allow class to be inherited. In that cases usually specific methods are allowed to be overridden but not all methods. I.e. in such classes I by default mark all methods as I.e. almost every class that I allow to be inherited has at least some final methods (though for most of the classes I forbid inheritance at all). And if such (non-final) class has some trivial getter and/or setter methods, they're most certainly final — because they're usually not the part of the stuff that is by design allowed to be overridden. Maybe it matters that I use Java not for my work but for my hobby projects. Never-the-less for me it's a usual thing: I make everything final by default and then consciously "unfinalize" some small parts. Getters and setters almost never get "unfinallized". (If I were a Lombok designer, I'd made getter and setters final by default, but I understand that now it might be too incompatible change.) |
@o-pikozh This sounds pretty convincing. Actually, this is just like what I always do concerning visibility (everything is as much private as possible). I don't do this w.r.t. finality as I never really thought about it, but I'd love when it was the default in Java - I mean everything is final unless marked as non-final (or there's a corresponding annotation on the whole class). But that train has sailed years ago. I'm rather sure that being restrictive is a (very) good thing, but I'm unsure if in this particular case it's good enough to warrant the effort. If there was a lombok.config option for making all etters final, I'd surely switch it on. Unfortunately, such an option requires a possibility to override it, which means an additional annotation parameter or a new annotation or alike... even more effort (implementation, maintenance and learning curve). Disclaimer: I'm not a lombok team member. |
Consider abstract base classes (for e.g. database objects => my use-case) as a real-world scenario. |
We also had that use case (database mapping classes for JCR). JCR also has a (very special) notion of inheritance. Thus, it becomes even more important to design the inheritance in Java well, especially as we allow plugins with customer-specific database sub-classes (i.e. code that we do not control). Joshua Bloch also says "Design for inheritance or else prohibit it" (Effective Java). Getter and setters are a typical example where you do not want any subclass to mess around with. Imagine a subclass overriding the getter for the unique ID. |
Agreed. Whenever possible, all methods should be either abstract or final. I'm just moving some fields up in the hierarchy. By making the getters final, I could ensure that there'll be no leftovers. |
"because I want to have sub-classes, but they are not allowed to override my getter/setter" it's an actual use-case. Just because you don't think so doesn't make it less important. I agree that a The final keyword in simple getter often appears in my value objects. Those value object often have a value and I want to be sure that value object subclasses don't return whatever they want. I want they initialize the value calling You can argue that I can simply declare my field Everything would be better if Java had properties like Kotlin, and getters and setters for those properties. |
Real use-case for you: |
This would be useful on Spring applications to autowire a field on an abstract class. |
Why on earth is this marked as parked instead of low hanging fruit? Surely, adding a single optional parameter to |
We asked for a use case and so far nothing that convinced us has been stated, except for the mockito one which isn't good enough. Hence, it's not low hanging fruit.
It's trivial. The PR would take us, what, 2 hours? And that mindset of yours ("just add everything that seems easy when a few folks ask for it!") means that 5 years from now, you type We've maintained this project for 12 years. We believe that our hesitance at introducing features 'just cuz it seems easy' plaus a significant role in this. Thus, whilst 'this is really easy to add' certainly helps, it's not, by any means, the most important consideration.
Where? I see the one about mockito, and a lot of abstract use cases which don't count.
And you think getting uppity with the core maintainers is going to make us go: "Oh, right, sorry about that, no problem, we'll get right on that?" You're not helping. We need use cases. Evidently that sentence is not obvious, I'll try to explain in a followup comment. |
We need use cases. I'll bat down a bunch of stuff stated as use cases in this issue which either aren't use cases or aren't convincing. @canemacchina wrote:
No, it's not an actual use case. Nobody sets out to say: "I wish to fasten this plate to this other plate by drilling holes through both and using a screw and bolt getup". That's completely pointless. Someone sets out to build a table, and as part of making the table, they run into the issue of needing to fasten two metal plates together. We need to hear about the table. If we have no experience with tables, we may even need to hear about the concept of 'serving dinner to big groups'. We'll keep asking for more details until we can fill in the rest of the 'chain' so to speak, and the above is so close to the 'how' and so far removed from the 'why', it is completely useless. We don't use everything you use, so we may come across as a bit dense. It's partly because we very well might be (case in point: Neither roel nor I use spring or mockito, coincidentally). And partly because we want everybody to understand why a feature is adding to WHY do you need to make subclasses that 'cannot override the getter'? If you simply intend to prevent overriding because you're aware that in general that is not a good idea, we're right there with you. But java is what it is, the solution is not to pepper @marcioggs wrote:
See, this is helping. A little bit: Now 'overwriting' the setter breaks auto-wiring and I can see how that may be non-obvious enough that the one doing the overriding just misses this. But this isn't even remotely close to good enough. This is good work though: Link to reputable tutorials (I consider baeldung reputable, I'd think most would) helps. Links to the web-hosted source control of major projects engaging in a certain style helps. To convince us something is universal boilerplate, you need to hit both beats in that sentence: The boilerplate part tends to be easy enough to show, the universal part - therein lies the rub. We may not use the frameworks you use daily. Even if we do, many lombok users won't. The feature needs to make sense either without mentioning those frameworks at all, or by mentioning such a varied batch of different frameworks where this comes up that it's effectively universal. @haelix888 wrote:
You bet. That counts and is helping push this request towards feasibility. I think I want more. I don't like mockito, but I'm pretty sure the java community has embraced it with gusto, but the problem here isn't motivating me, roel, or anyone else to write the PR (that is the easy part). Hence, the only thing that matters is community understanding. |
@rzwitserloot I think someone also mentioned abstract classes when used with DI frameworks and whatnot, or just when proving things for external usage and the like. TBH, I understand that these might not be soooooo compelling use-cases, but I see there is already other things of the same sort on Lombok (makeFinal in FieldDefaults for example) which are arguably more useful, but may end up not so useful of you can override the getters or something. So I think we could consider this a low-hanging fruit by that token? I'm only following this issue because I once thought I needed something like that, so I guess people are running into use-cases? (This other person's attitude is totally not acceptable, but I decided to address your other points regarding not considering adding this feature). I can provide a PR for this if it makes it easier to justify adding it? (I never bothered to make it happen after I moved in from the project where I needed it, but I can find some time this weekend or the next, I think) |
@andrebrait wrote:
@Getter(access = AccessLevel.PACKAGE) class Example {
@Getter String foo;
int bar;
} is mostly irrelevant. Why would you write that? I'm not even usre what happens if you do this, but hopefully it is clear that one can plausibly say: That field-level annotation does nothing, and one can also plausibly say: That field level annotation pushes the access level back to the default, which is Add This isn't a showstopper. Of course not. Merely a complication; we'll find some acceptable answer and move on if we decide to add this feature. But it is exactly that kind of thing that makes me so very miffed at folks like @jennierose who act like we owe them something. No, we don't, and no, this stuff is much, much, much more complicated than you can evidently comprehend, so please stop suggesting we're not implementing this out of spite or laziness. We're hesitant to keep lombok easy to use, and we feel keeping such 'what-ifs' to a minimum is an important aspect of keeping it easy to use. Point is, this is by no means low hanging fruit, and isn't comparable to
I'd like to specifically know which ones and what it looks like. If it's just about preventing your own programming team from making an obvious mistake, that wouldn't be very convincing. If the
Much appreciated! But this one is a bit of niche issue - it's so easy to make, virtually all the work lies in writing the docs, the tests, and signing up to the adjustment in lombok's learning curve, which new committers probably can't help us with. If you'd like to in order to have a few lines in the code base, by all means. But wait for sign-off. I'm not convinced yet, it's something @rspilker and I will need to discuss (Unless @rspilker is quite opposed to this, an endorsement from a major contributor would push this over the line). |
@rzwitserloot got your points. As for the code sample, I would assume the field-level one has precendence over the class-level one, IIRC. I think I've done that sort of thing before and it's what I remember seeing when I did #2513. |
Yes, field-level annotations have precedence, I think that's what almost everybody would assume, but do they act like annotations and thus, I would optimally want to rewrite |
Nice to see some "progress" for this feature request. :) I must admint I don't understand the confusion. I get your point with "50 parameters" for the @Getter/@Setter annotations, but I think it is a bit exaggerated. I still think support for the "final" keyword in the byte code for getters/setters generated by Lombok would be a great addition. In my case I use final methods (incl. getters/setters) in abstract classes in order to point out that those methods are not safe/destined for overriding. Even in non-abstract classes this approach is useful. Thanks for your patience and keep being "cautious" regarding change requests. Cheers! |
Happy to see some action here and also that my "use case" with Mockito is deemed the most material so far (albeit not reaching critical mass). FWIW I though I might add that final also has (almost surely) an bearing on performance also and Getters being so widespread and conveniently generated with Lombok, this fact is not without significance. |
It's about this situation: @Getter(access = AccessLevel.PROTECTED)
class Example {
@Getter(makeFinal = true) String foo;
String bar;
} If you don't see how it this snippet is non-obvious about what that does (is it
We are in this to make a nice tool that our users love. But when it comes to opining about maintainability, the only opinions that matter are those of contributors that earned the right to talk. You haven't. That's okay - feedback is welcome and I don't expect you to know which kinds of feedback are more useful than others. Hopefully you'll allow me the freedom to state that certain feedback ends up not being actionable!
Hotspot probably makes this point moot. If someone shows me JMH measurements that indicate it isn't, that would be another solid use case and would push this over the edge. |
Please consider (benefits/downsides) If it's part of |
I would clearly expect this to be |
@o-pikozh Yes, and someone else in this thread indicated they clearly expect that to be |
I would expect it to be public. The reason being that it's what acess is set to in the field-level annotation. Default values are still values. I wouldn't expect a cascade effect here. So this is clearly an issue, like @rzwitserloot is demonstrating. As for moving that to I'd think a final getter but non-final setter (or vice-versa) to be an extremely rare use-case. Though I guess we'd still have the same issue of "expected behavior" when using that with class and field-level annotations. The only alleviating thing is a note on the documentation for Accessors which states that field-level doesn't cascade with the class-level annotation. Otherwise, it could look just as confusing? E.g.: @Accessors(makeFinal = true)
public class Foo {
@Accessors(prefix="f")
@Getter
private final int Bar;
} Would that be a problem @rzwitserloot @rspilker? |
The result there would be a non-final getter, according to the documentation. Just commenting on the fact it could be confusing the same way adding it to Getter would. |
From the documentation:
|
Have at it: https://projectlombok.org/download-edge |
Add
boolean makeFinal() default false
toGetter
andSetter
.Though it's already discussed here (discusstion start is here) and will, probably, be "won't fix" anyway — but I consider it useful in some rare cases when getter and/or setter is needed to be explicitly final (not for marking most of getters/setters as final).
As I haven't found this wish in this issue-tracker, I took the liberty to repost it here (at least: to group all requests in one place) — please, feel free to close it immediately, if it's still a no-way.
The text was updated successfully, but these errors were encountered: