-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 > >
