On Mon, Dec 8, 2008 at 10:26 AM, Raymond Feng <[EMAIL PROTECTED]> wrote: > 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.
+1 > > 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. > +1 for Good judgmet, I guess it's more like trying to avoid adding a dependency on another module just to use a constant. >> >>> - 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 >>> >> >> > -- Luciano Resende Apache Tuscany, Apache PhotArk http://people.apache.org/~lresende http://lresende.blogspot.com/
