Syntactical code clean

[GoogleCodeExporter]

The current code base is mostly formatted the UMD way - I'd like to apply a
wide range code clean once to reduce whitespace diffs in the future.

I think of
1) format all sources with the eclipse formatter
http://code.google.com/p/piccolo2d/source/browse/piccolo2d.java/trunk/core/src/b
uild/conf/eclipse-formatting-conventions.xml
2) let eclipse organise the imports
3) let eclipse add 'final' wherever possible
4) add serialVersionUIDs
5) put the cleanup rules into an eclipse profile
6) add mandatory (for eclipse users only) eclipse code templates

Original issue reported on code.google.com by mr0...@mro.name on 28 Jul 2009 at 10:10

[GoogleCodeExporter]

I disagree with #3 for the reasons I've given you before.

Original comment by allain.lalonde on 28 Jul 2009 at 12:02

[GoogleCodeExporter]

Let me refer to
http://books.google.de/books?id=n4jmGp2EO1YC&pg=PA43&lpg=PA43&dq=final&source=bl
&ots=j73YVpCe7i&sig=vIkMEfh0kW6XMlh7ubwtXxr-dK4&hl=de&ei=kOpuSvH_M8eOsAaX9aSXBQ&
sa=X&oi=book_result&ct=result&resnum=4
- can't say it any better.

Original comment by mr0...@mro.name on 28 Jul 2009 at 12:24

[GoogleCodeExporter]

I presume that (3) doesn't entail making any classes or non-private methods 
final
(none should be).  What reasons has allain.lalonde given for not using 'final' 
where
possible?  They don't appear in this thread.  I agree that final parameters and
variables (and fields where possible) are safer than the mutable form, and that 
it is
worth the increase in verbosity.

Original comment by samrr...@gmail.com on 28 Jul 2009 at 12:42

[GoogleCodeExporter]

right - only changing non-public items. Has to pass the clirr test for binary
compatibility to 1.2.1 both before and afterwards.

Allain had aches concerning code-compactness, favoured code metrics a la
checkstyle/pmd/findbugs and questioned the benefit as it might suggest false 
security.

Original comment by mr0...@mro.name on 28 Jul 2009 at 1:04

[GoogleCodeExporter]

(copied form mailing list)

final on parameters protects us from writing to them from within 
methods. 

FindBugs and PMD (I think) both flag this behaviour as problematic and 
catch many others in the process. 

If, and it's a big one, we target reducing the # of warnings being 
flagged by these tools, bugs introduced by not having final get caught 
anyway (along with countless others). 

I think adding final in a context where FindBugs failures breaks the 
build is unnecessary. 

That said, adding final to parameters doesn't hinder anything, it just 
adds verbosity for something that FindBugs and PMD would both disallow 
anyway. 

Not dead set against it, and the code will end up higher quality in 
the end no matter, I'm just stating my dislike for blanket covering a 
codebase with final when it's not necessary with a strict development 
process. 

Just my 2 cents. 

Original comment by allain.lalonde on 28 Jul 2009 at 2:06

[GoogleCodeExporter]

If we already have a process in place that flags reassignment to variables and
arguments (as allain suggested above), then I think inserting 'final' is 
unnecessary.

Original comment by samrr...@gmail.com on 28 Jul 2009 at 4:05

[GoogleCodeExporter]

I thought I did apply the Eclipse formatter at one point in the past?  
Checkstyle
doesn't show many whitespace errors.

I would advise to be careful with the final parameter stuff, as many methods in
Piccolo2D modifiy the input parameters.

I don't typically use an IDE, so the automatic format-on-cleanup is going to be 
lost
on me.

Original comment by heue...@gmail.com on 28 Jul 2009 at 4:35

[GoogleCodeExporter]

@Michael: you did? the better! So there won't be many ws diffs.

@All:

A process in place is a good thing to have. But if that leads to not using the
compiler to detect flaws I think it's misused.

Again: I talk about making implicit final private stuff explicitly final so the
compiler can check. Who else wants to check this may well do so.

I just don't see the point why we shouldn't use the compiler to check this.

Please show me the code metric that tells me that I changed a before final field
accidently into a mutated one. And even if there were such a mindreading tool - 
I'd
prefer the compiler shouting at me. Requires no plugin into whatever editor one 
may use.

The code becomes more explicit, to both, man and machine.

I'll do it in ~2h from now.

Original comment by mr0...@mro.name on 28 Jul 2009 at 5:04

[GoogleCodeExporter]

done it, see r599

local mvn build ok, hudson struggles with the checkout.

@Michael: can you clirr this version against 1.2.1?

Original comment by mr0...@mro.name on 28 Jul 2009 at 8:13

[GoogleCodeExporter]

BTW, r599 also added the Eclipse Code Clean Profile
http://code.google.com/p/piccolo2d/source/browse/piccolo2d.java/trunk/core/src/b
uild/conf/eclipse-code-clean.xml

or for those who like it a bit more visual:
http://www.getdropbox.com/gallery/965005/2/piccolo2d/code-clean?h=334773

Original comment by mr0...@mro.name on 28 Jul 2009 at 8:15

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by heue...@gmail.com on 21 Oct 2009 at 3:55

• Changed state: Verified