Voting and discussion are closed for now. I'll count up the votes and post a summary soon.
-Kirk On Thu, Apr 22, 2021 at 7:54 AM Bruce Schuchardt <bru...@vmware.com> wrote: > Kirk speaks my mind. -1 on this requirement > > On 4/21/21, 11:29 AM, "Kirk Lund" <kl...@apache.org> wrote: > > -1 as I've already explained in my DISCUSS thread for this topic, I > don't > think final gives sufficient benefit for local variables or method > parameters because it only prevents reassigning object pointers and > primitive values. > > I favor individuals making a personal decision on whether or not they > want > to use 'final' in this way without requiring others to do the same > (and NOT > requiring this use of 'final' in PR reviews, although "suggesting" is > ok). > > Note that JDK and all other libraries I looked through do not use > final in > this way. > > *I'm willing to change my vote to -0 if*: > > 1. we qualify the requirement to only apply to method and > constructor > parameters (not local vars) > 2. we find a way to introduce automation for enforcement (such as > we do > with RAT for license headers) > > *I'm willing to change my vote to +1 if *we decide to require 'final' > parameters (and even local vars) as part of a bigger shift towards > improving code quality by embracing Clean Code (ala Bob Martin): > > 1. we commit to developing a robust and thorough coding standard > that > includes requiring true unit tests and passing in dependencies via > class > constructors or methods > 2. we revisit our DOD and enforce it in PR reviews > > I recommend that contributors use IntelliJ inspections (or comparable > guideline for anyone who doesn't want to use IntelliJ) that will > assist us > in avoiding and fixing long methods when writing new methods and when > editing/refactoring existing methods (see below) > Method metrics (see IntelliJ > Preferences > Editor > Inspections): > > - Method with too many parameters > - Overly complex method > - Overly long method > - Overly nested method > > The inspections can be customized for Geode as long as the values > aren't > increased to the point of it being useless. For example, we would have > to > agree as a community what the "maximum number of parameters" is for the > "Method with too many parameters" inspection. > > We could even export an inspections profile to be stored in the Geode > repo > and used by contributors. > > -Kirk > > On Wed, Apr 14, 2021 at 12:55 PM Kirk Lund <kl...@apache.org> wrote: > > > Our coding standard and our Design Decisions does NOT require using > final > > on parameters and local variables. > > > > Please do NOT request that I add or keep final on parameters or local > > variables unless the community votes and decides that every > parameter and > > local variable should require the final keyword. I coded this way in > the > > past and I found that it resulted in noisy code with no benefit. We > can > > argue about using this keyword all you want but the fact is I'm > against it, > > and I will not embrace it unless this community decides that we need > to use > > it. > > > > Using final on instance or class fields does have a concurrency > benefit > > and I support that only. > > > > If you want to add final to every single parameter and local var in > the > > Geode codebase, then now is your chance to vote on it. Please vote. > > > > Thanks, > > Kirk > > > > > >
