As we can expect, some of the rules are subjective. I don't see that as a
big issue. Any attempt to make the code less error-prone and more readable
should be appraised and appreciated.
BTW, a few comments inline too :-).
Thanks,
Raymond
--------------------------------------------------
From: "Simon Nash" <[EMAIL PROTECTED]>
Sent: Sunday, December 07, 2008 9:01 AM
To: <[email protected]>
Subject: Re: Trunk Code clean up
Luciano Resende wrote:
I have noticed that Mark has been doing a great job helping us
cleaning up the 2.0 code base [1], thanks a lot Mark.
Considering others might want to help, below is a list of possible
items to look when reviewing the code, we could probably grow this
into a Tuscany Code Guideline.
>
Good suggestions, but I'm not sure about a few of these. See comments
below.
Also, using a tool such as PMD (or eclipse PMD plugin) might help.
Clean code
- use correct visibility, private, default, public, avoid protected
>
I don't see anything wrong with using protected. What else would we
use for methods that should only be visible to subclasses?
"protected" should be only used when it's necessary for subclassing :-)
- make methods static if not using object state
>
I would not make this a hard and fast rule. IMO the main reason for
making a method static is to allow ti to be called when an instance
is not available.
- make sure javadoc is in sync or remove that javadoc
- no javadoc on overridden methods
- test cases in same package to avoid having to over-open access to
methods
- don't create artificial dependencies by using constants from another
module
>
This could lead to a lot of duplicate constant definitions in different
modules - not necessarily a good thing.
We should use good judgment for public constants. It's similar to utility
classes.
- don't extend/implement a 'constant' interface
>
Why not?
A better way could be using "import static". I think implementing constant
interface is a workaround before "import static" is added for Java 1.5.
- avoid creating another private layer over a public interface/spi
- remove old code, don't leave it commented out, very confusing
>
Sometimes there is good reason for leaving it there. This can be
very helpful when making a tentative change that is likely to be
revisited in the future.
- use functional programming names for functions that convert an object
- add javadoc to private methods
- review class javadoc and make sure it's accurate
- inline methods used only once, or make them clean static functions
>
I don't see any problem with an out-of-line non-static method that
happens to be used only once. If used appropriately, these can
make code easier to read.
Use good balance here. Organize the methods by meaningful functional blocks.
Avoid methods that have pages of code and one-time methods that only have
one or two lines.
Simon
- put utility methods in a Util class with package visibility
- use static imports
- use scoped variables
- no stars in OSGi exports
- correct use of generics, see the effective Java book
Unit/Integration Tests
- put comments in test cases
- review test cases and make sure they're included in the build
- use junit 4 only, check correct use of @BeforeClass or @Before
- use static imports for assert statements
Formatting
- use Tuscany eclipse code style/formatter
- no tabs
- no excessive line wrapping
[1]
http://www.mail-archive.com/search?q=mcombellack&[EMAIL PROTECTED]&start=0
