I see. I imagine we could have this discussion every time a new developer comes on board. :-)
So, I see the points they're making, and I don't necessarily disagree with all of them ipso facto. I, personally, always try to make instance variables final when appropriate. However, I also think that each developer knows best the intent of his code, so if we want to put this "always use final when possible" practice into place, the best way to do that is to train developers to use it. Put it in the developer guidelines for contributors and committers, etc. This blanket changing of dozens of files that the changer has no knowledge of and isn't even reviewing individually concern me. And that doesn't even address the fact that these changes increase the Checkstyle errors. The additional of the six characters per variable (in a method with many parameters, that's a lot of characters) causes some lines of code to exceed the 120 character limit and trigger Checkstyle. Finally, even /if/ I were to agree to blanket changes like this, I think they were done at the wrong time. At $work, we have a policy about such changes: Blanket reformats and policy changes are always made at the /beginning/ of a development cycle (immediately after a release) as opposed to at the /end/ of a development cycle (immediately before a release, as GG has done with MANY changes in the last few days). While such changes /shouldn't/ change program logic, tools (Eclipse) are only as good as the people who write them (don't get me started on my opinion of Eclipse) and can make mistakes. We shouldn't be doing things like this right before a release. My $0.02. Nick On Jul 9, 2013, at 7:14 PM, Ralph Goers wrote: > This topic was covered in an email thread that started with this email - > http://mail-archives.apache.org/mod_mbox/logging-log4j-dev/201301.mbox/%3CCACZkXPwsDyD-X%2B5wHuq4X26wzCZnsHJ%3Ds5rLJNKe71XYyg6LfA%40mail.gmail.com%3E. > Unfortunately I don't know how to link the whole thread from the apache > archives but you can find it at http://marc.info/?t=134978818500007&r=1&w=2. > > I happen to agree with you, as you will see from the thread. I find it more > annoying that many, many files were changed at once with no warning. I also > suspect Gary does this through something automated in Eclipse, which means he > doesn't see the comments that say "don't do that". > > Ralph > > On Jul 9, 2013, at 4:41 PM, Nick Williams wrote: > >> Gary, >> >> Earlier today you committed r1501104 with the following comment: >> >>> Add final modifier to private fields. >>> Add final modifier to method parameters. >>> Add final modifier to local variables. >> >> I've seen you do similar things before, too. Can you explain a little bit >> why you do this? There's certainly no compiler value--the compiler for a >> long time has been capable of determining whether a variable is ever changed >> and optimizing appropriately. All it seems to do to me is clutter the >> code...significantly. It makes the code harder to read, IMO. >> >> Can you justify the intentional mass change of this nature? >> >> Thanks, >> >> Nick >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> >
