I wouldn't recommend Lombok for production code, primarily because it generates code. Generated code occasionally leads to unintended consequences or incompatibilities with newer Java versions. Additionally, it requires as 3rd-party dependency to gain these capabilities rather than being part of the Java language itself. I'd rather Java include appropriate features for these 2 concerns, i.e. val & var, much like Kotlin, to more succinctly express intent. Even though Java now has var, it is different than Kotlin's var.
I only use Lombok for testing and demoing purposes. -j ________________________________ From: Kirk Lund <kl...@apache.org> Sent: Wednesday, April 21, 2021 12:59 PM To: dev@geode.apache.org <dev@geode.apache.org> Subject: Re: [VOTE] Requiring final keyword on every parameter and local variable I'm interested in https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprojectlombok.org%2F&data=04%7C01%7Cjblum%40vmware.com%7Cebf313cf7061468b6e8508d904fff633%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637546319662043634%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=yHceAr%2FbQtVTQAmSWxkNYkctPFP8nG6fph0QWLCRaDA%3D&reserved=0 and I remember John Blum using it on some projects. I would like to study it and do some perf testing on it before supporting it for Geode though. In general, I don't like generated code and it looks like at least some of the features involve generated code -- that's the only potential downside for me. -Kirk On Mon, Apr 19, 2021 at 12:57 PM Owen Nichols <onich...@vmware.com> wrote: > Any interest in adopting > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprojectlombok.org%2F&data=04%7C01%7Cjblum%40vmware.com%7Cebf313cf7061468b6e8508d904fff633%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637546319662043634%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=yHceAr%2FbQtVTQAmSWxkNYkctPFP8nG6fph0QWLCRaDA%3D&reserved=0 > in Geode? Lombok > allow a more readable "val" vs "var" syntax instead of "final" vs "" which > could make it easier to do the right thing without "increasing visual > noise". > > On 4/19/21, 12:40 PM, "Dale Emery" <dem...@vmware.com> wrote: > > My test for whether enforce a guideline in a PR review is: Would I be > willing to automate enforcing it in CI? > > I am -0 on this particular guideline. > > IntelliJ offers two competing inspections for Java coding style: > > * Local variable or parameter can be final > * Unnecessary 'final' on local variable or parameter > > Both of these are (I think) disabled by default in IntelliJ. And > neither satisfies my “I’d be willing to automate enforcing it” test. > > Dale > > From: Mark Hanson <hans...@vmware.com> > Date: Monday, April 19, 2021 at 11:08 AM > To: dev@geode.apache.org <dev@geode.apache.org> > Subject: Re: [VOTE] Requiring final keyword on every parameter and > local variable > Hi Jake, > > I agree with everyone's point about final being a useful, I just don't > find it useful enough to do anything manually across the code base at this > point. > > I believe first in foremost in no code warnings. Intellij warns about > variables that can be final, so I make them final as it finds them. It is > very rare that I am writing new code at this point. I have spent the last > year bug fixing other people's code. From my standpoint, original intent > is largely moot. So, for me, the question is do I go through code that is > not mine and mark it all as final (where appropriate)? Sure, in code that I > touch. In code the rest of the codebase, I think there are bigger fish to > fry before we get to final. > > It seems like the larger portion of the consensus is to recommend that > variables are marked final (where appropriate) as we find them or create > them. > That seems like the going forward approach consensus. > > I will be blunt though. This seems nitpicky *compared* to the number > of code warnings there are and the fact that people are not actively fixing > all of the warnings. > If we don't come to the same consensus about all warnings this seems > like painting a rusted car, sure it makes it all look the same, but does > very little for the underlying problems. Now to undermine my own argument a > little, I think that especially in release blockers, we want to touch as > little code as possible, so I am flexible there. > > I would also like to agree with Udo about final really not being very > good compared to Mutable and Immutable objects in other languages. > > Thanks, > Mark > > > On 4/15/21, 7:49 PM, "Udo Kohlmeyer" <u...@vmware.com> wrote: > > @jake, you are correct, I did miss the LOCAL variable out of the > subject. :face_palm: > > Yes, adding "final" to LOCAL variables will be HUGELY beneficial > in this regard. Have we seen any performance improvement after having > refactored this method? Have you been able to measure any improvements? > > --Udo > > On 4/15/21, 1:19 PM, "Jacob Barrett" <jabarr...@vmware.com> wrote: > > > > > On Apr 14, 2021, at 7:46 PM, Udo Kohlmeyer <u...@vmware.com> > wrote: > > @Jake the idea of smaller methods is great and we should > ALWAYS strive for that. But that argument is completely irrelevant in this > discussion. As making method arguments final does not naturally guide a > developer to creating smaller methods. Nor does a smaller method mean it > can/will be jitted. Too many factors (to discuss here) are part of that > decision, also it is not relevant in this discussion. But more on that > topic read THIS. > > The original subject is in regards to parameters and local > variables. > > Irrelevant is certainly an opinion you are welcome to have but > let me challenge you. Goto DistributedCacheOperation._distribute(). > > First challenge, look at around line 333: > boolean reliableOp = isOperationReliable() && > region.requiresReliabilityCheck(); > > Without scrolling do you see that variable used? Nope, because > it is first used on line 439, ~100 lines away. Does it mutate between > there, well I can search for all uses and find out or I could be nice to > the next person and intend for it to never mutate by adding final. Intent > communicated! > > Second challenge, mark all the local variables in the method > as final. Now make it compile without introducing more mutable variables. > At the end of this journey you will have about a dozen unit testable > methods and a _distribute method that goes from ~370 lines to ~90 with no > mutable local variables. > > I argue it is relevant as good guardrail for writing good > code. While we should ALWAYS strive for it we don’t. Every little nudge > helps. > > > -Jake > > > >
