You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by Alex Grönholm <al...@nextday.fi> on 2008/10/29 18:44:30 UTC

Re: Code conventions

David Blevins kirjoitti:
>
> On Oct 16, 2008, at 3:48 PM, Alex Grönholm wrote:
>
>> What I'm saying is that we need a written set of guidelines so that 
>> freshly contributed code wouldn't have to be constantly reformatted. 
>> A good sized set of example code files would go a long way towards 
>> this goal. If you agree with me on this, we could set up a Wiki page 
>> for contributors where all the necessary instructions and guidelines 
>> would go.
>
> Now that you mention it recall that Maven had a pretty cool approach 
> to this with a basic doc and then actual IDE config files for Eclipse 
> and Intellij that people could just install.  
> http://maven.apache.org/developers/conventions/code.html
>
> We could do something like that.
>
> -David
>
I have created my suggestions for Eclipse code formatting/cleanup 
definitions and code templates.
They can be found at:
http://paradoxx.dyndns.org/~demigod/openejb_eclipse_formatter.xml
http://paradoxx.dyndns.org/~demigod/openejb_eclipse_cleanup.xml
http://paradoxx.dyndns.org/~demigod/openejb_eclipse_codetemplates.xml

I would prefer to have these eventually hosted elsewhere.

Here are the main points of the formatting definitions.

Formatter:
- max line width is 100 characters
- indentation is 4 spaces
- javadocs are formatted

Cleanup:
- always use braces with control statements (I actually disagree with 
this, and it's not even consistently used in existing code)
- removes unused imports and local variables
- removes "this" qualifier for non-static method/field accesses (where 
possible)
- adds missing @Override/@Deprecated annotations
- removes unnecessary casts
- removes trailing whitespace
- corrects indentation

Code templates:
- for newly create .java files, inserts the ASL comment to the top


Re: Code conventions

Posted by Alex Grönholm <al...@nextday.fi>.
Daniel S. Haischt kirjoitti:
> just a suggestion ;)
>
> it is possible to export/import a distinct set of preferences as
> well... In that case everything would
> be self-contained.
>
>   
We could probably join the three files into one.
Still, no one has yet commented on the actual content of these proposed 
conventions. Can I assume that silence means approval?
> On Thu, Oct 30, 2008 at 1:23 PM, Alex Grönholm
> <al...@nextday.fi> wrote:
>   
>> Daniel S. Haischt kirjoitti:
>>     
>>> it's as well possible to export your complete Eclipse preferences
>>> AFAIK which should then
>>> include the source code style settings.
>>>
>>>
>>>       
>> I don't think anyone wants to import ALL of my Eclipse preferences.
>>     
>>> The preferences then can be imported per workspace...
>>>
>>>
>>> On Thu, Oct 30, 2008 at 11:33 AM, Alex Grönholm
>>> <al...@nextday.fi> wrote:
>>>
>>>       
>>>> Jacek Laskowski kirjoitti:
>>>>
>>>>         
>>>>> On Thu, Oct 30, 2008 at 9:24 AM, Alex Grönholm
>>>>> <al...@nextday.fi> wrote:
>>>>>
>>>>>
>>>>>
>>>>>           
>>>>>> AFAIK you have to import them manually through project settings -> Java
>>>>>> Code
>>>>>> Style.
>>>>>>
>>>>>>
>>>>>>             
>>>>> I wonder how we could enforce them before commit? There's a maven
>>>>> plugin for checkstyle, but I wouldn't like having code styles in more
>>>>> than one place. Is there a way to use the eclipse code style in a
>>>>> checkstyle-based plugin? Could it be converted to support IDEA,
>>>>> Netbeans?
>>>>>
>>>>> Jacek
>>>>>
>>>>>
>>>>>           
>>>> I meant these to be recommendations rather than something to be strictly
>>>> enforced. Those definition files are more like a convenience that makes
>>>> it
>>>> easier for Eclipse users to follow the code conventions for this project.
>>>> If
>>>> anyone manages to produce similar files for NetBeans or IDEA, so much the
>>>> better.
>>>>
>>>>
>>>>         
>>     


Re: Code conventions

Posted by "Daniel S. Haischt" <da...@googlemail.com>.
just a suggestion ;)

it is possible to export/import a distinct set of preferences as
well... In that case everything would
be self-contained.

On Thu, Oct 30, 2008 at 1:23 PM, Alex Grönholm
<al...@nextday.fi> wrote:
> Daniel S. Haischt kirjoitti:
>>
>> it's as well possible to export your complete Eclipse preferences
>> AFAIK which should then
>> include the source code style settings.
>>
>>
>
> I don't think anyone wants to import ALL of my Eclipse preferences.
>>
>> The preferences then can be imported per workspace...
>>
>>
>> On Thu, Oct 30, 2008 at 11:33 AM, Alex Grönholm
>> <al...@nextday.fi> wrote:
>>
>>>
>>> Jacek Laskowski kirjoitti:
>>>
>>>>
>>>> On Thu, Oct 30, 2008 at 9:24 AM, Alex Grönholm
>>>> <al...@nextday.fi> wrote:
>>>>
>>>>
>>>>
>>>>>
>>>>> AFAIK you have to import them manually through project settings -> Java
>>>>> Code
>>>>> Style.
>>>>>
>>>>>
>>>>
>>>> I wonder how we could enforce them before commit? There's a maven
>>>> plugin for checkstyle, but I wouldn't like having code styles in more
>>>> than one place. Is there a way to use the eclipse code style in a
>>>> checkstyle-based plugin? Could it be converted to support IDEA,
>>>> Netbeans?
>>>>
>>>> Jacek
>>>>
>>>>
>>>
>>> I meant these to be recommendations rather than something to be strictly
>>> enforced. Those definition files are more like a convenience that makes
>>> it
>>> easier for Eclipse users to follow the code conventions for this project.
>>> If
>>> anyone manages to produce similar files for NetBeans or IDEA, so much the
>>> better.
>>>
>>>
>
>

Re: Code conventions

Posted by Alex Grönholm <al...@nextday.fi>.
Daniel S. Haischt kirjoitti:
> it's as well possible to export your complete Eclipse preferences
> AFAIK which should then
> include the source code style settings.
>
>   
I don't think anyone wants to import ALL of my Eclipse preferences.
> The preferences then can be imported per workspace...
>
>
> On Thu, Oct 30, 2008 at 11:33 AM, Alex Grönholm
> <al...@nextday.fi> wrote:
>   
>> Jacek Laskowski kirjoitti:
>>     
>>> On Thu, Oct 30, 2008 at 9:24 AM, Alex Grönholm
>>> <al...@nextday.fi> wrote:
>>>
>>>
>>>       
>>>> AFAIK you have to import them manually through project settings -> Java
>>>> Code
>>>> Style.
>>>>
>>>>         
>>> I wonder how we could enforce them before commit? There's a maven
>>> plugin for checkstyle, but I wouldn't like having code styles in more
>>> than one place. Is there a way to use the eclipse code style in a
>>> checkstyle-based plugin? Could it be converted to support IDEA,
>>> Netbeans?
>>>
>>> Jacek
>>>
>>>       
>> I meant these to be recommendations rather than something to be strictly
>> enforced. Those definition files are more like a convenience that makes it
>> easier for Eclipse users to follow the code conventions for this project. If
>> anyone manages to produce similar files for NetBeans or IDEA, so much the
>> better.
>>
>>     


Re: Code conventions

Posted by "Daniel S. Haischt" <da...@googlemail.com>.
it's as well possible to export your complete Eclipse preferences
AFAIK which should then
include the source code style settings.

The preferences then can be imported per workspace...


On Thu, Oct 30, 2008 at 11:33 AM, Alex Grönholm
<al...@nextday.fi> wrote:
> Jacek Laskowski kirjoitti:
>>
>> On Thu, Oct 30, 2008 at 9:24 AM, Alex Grönholm
>> <al...@nextday.fi> wrote:
>>
>>
>>>
>>> AFAIK you have to import them manually through project settings -> Java
>>> Code
>>> Style.
>>>
>>
>> I wonder how we could enforce them before commit? There's a maven
>> plugin for checkstyle, but I wouldn't like having code styles in more
>> than one place. Is there a way to use the eclipse code style in a
>> checkstyle-based plugin? Could it be converted to support IDEA,
>> Netbeans?
>>
>> Jacek
>>
>
> I meant these to be recommendations rather than something to be strictly
> enforced. Those definition files are more like a convenience that makes it
> easier for Eclipse users to follow the code conventions for this project. If
> anyone manages to produce similar files for NetBeans or IDEA, so much the
> better.
>

Re: Code conventions

Posted by Alex Grönholm <al...@nextday.fi>.
Jacek Laskowski kirjoitti:
> On Thu, Oct 30, 2008 at 9:24 AM, Alex Grönholm
> <al...@nextday.fi> wrote:
>
>   
>> AFAIK you have to import them manually through project settings -> Java Code
>> Style.
>>     
>
> I wonder how we could enforce them before commit? There's a maven
> plugin for checkstyle, but I wouldn't like having code styles in more
> than one place. Is there a way to use the eclipse code style in a
> checkstyle-based plugin? Could it be converted to support IDEA,
> Netbeans?
>
> Jacek
>   
I meant these to be recommendations rather than something to be strictly 
enforced. Those definition files are more like a convenience that makes 
it easier for Eclipse users to follow the code conventions for this 
project. If anyone manages to produce similar files for NetBeans or 
IDEA, so much the better.

Re: Code conventions

Posted by Jacek Laskowski <ja...@laskowski.net.pl>.
On Thu, Oct 30, 2008 at 9:24 AM, Alex Grönholm
<al...@nextday.fi> wrote:

> AFAIK you have to import them manually through project settings -> Java Code
> Style.

I wonder how we could enforce them before commit? There's a maven
plugin for checkstyle, but I wouldn't like having code styles in more
than one place. Is there a way to use the eclipse code style in a
checkstyle-based plugin? Could it be converted to support IDEA,
Netbeans?

Jacek

-- 
Jacek Laskowski
Notatnik Projektanta Java EE - http://www.JacekLaskowski.pl

Re: Code conventions

Posted by Alex Grönholm <al...@nextday.fi>.
Jacek Laskowski kirjoitti:
> On Wed, Oct 29, 2008 at 6:44 PM, Alex Grönholm
> <al...@nextday.fi> wrote:
>
>   
>> I have created my suggestions for Eclipse code formatting/cleanup
>> definitions and code templates.
>>     
> ...
>   
>> I would prefer to have these eventually hosted elsewhere.
>>     
>
> It could be in the repo under eclipse or so, but I don't know how to
> let Eclipse know about this location so it can use it.
>
> Jacek
AFAIK you have to import them manually through project settings -> Java 
Code Style.

Feedback regarding the proposed rules would also be welcome.

Re: Code conventions

Posted by Jacek Laskowski <ja...@laskowski.net.pl>.
On Wed, Oct 29, 2008 at 6:44 PM, Alex Grönholm
<al...@nextday.fi> wrote:

> I have created my suggestions for Eclipse code formatting/cleanup
> definitions and code templates.
...
> I would prefer to have these eventually hosted elsewhere.

It could be in the repo under eclipse or so, but I don't know how to
let Eclipse know about this location so it can use it.

Jacek

-- 
Jacek Laskowski
Notatnik Projektanta Java EE - http://www.JacekLaskowski.pl

Re: Code conventions

Posted by David Blevins <da...@visi.com>.
On Oct 30, 2008, at 4:40 PM, Alex Grönholm wrote:

> David Blevins kirjoitti:
>>
>> On Oct 30, 2008, at 4:03 PM, Alex Grönholm wrote:
>>
>>> Updated openejb_eclipse_formatter.xml to 120 char max line length.  
>>> Can these files be hosted in the wiki as attachments of some sort?
>>
>> They should be able to.  There should be an "Attachments" tab  
>> between "Edit" and "Info"
>>
>> -David
>>
> Alright. Next we just need a Wiki page that contains all the  
> necessary instructions for contributing. If you could start on that  
> page, I'll add the code conventions there if you want.

We've got this growing page that contains good information for  
contributors http://openejb.apache.org/tips-and-suggestions.html

We could add the conventions there if it's not too large or we could  
add it as a separate page and link to that page.

-David


Re: Code conventions

Posted by Alex Grönholm <al...@nextday.fi>.
David Blevins kirjoitti:
>
> On Oct 30, 2008, at 4:03 PM, Alex Grönholm wrote:
>
>> Updated openejb_eclipse_formatter.xml to 120 char max line length. 
>> Can these files be hosted in the wiki as attachments of some sort?
>
> They should be able to.  There should be an "Attachments" tab between 
> "Edit" and "Info"
>
> -David
>
Alright. Next we just need a Wiki page that contains all the necessary 
instructions for contributing. If you could start on that page, I'll add 
the code conventions there if you want.

Re: Code conventions

Posted by David Blevins <da...@visi.com>.
On Oct 30, 2008, at 4:03 PM, Alex Grönholm wrote:

> Updated openejb_eclipse_formatter.xml to 120 char max line length.  
> Can these files be hosted in the wiki as attachments of some sort?

They should be able to.  There should be an "Attachments" tab between  
"Edit" and "Info"

-David


Re: Code conventions

Posted by Alex Grönholm <al...@nextday.fi>.
David Blevins kirjoitti:
>
> On Oct 30, 2008, at 1:34 PM, Dain Sundstrom wrote:
>
>> On Oct 30, 2008, at 1:09 PM, Alex Grönholm wrote:
>>
>>> Dain Sundstrom kirjoitti:
>>>> On Oct 29, 2008, at 10:44 AM, Alex Grönholm wrote:
>>>>
>>>>> Formatter:
>>>>> - max line width is 100 characters
>>>>
>>>> I prefer 120, but as long as it is only suggestion it isn't a big 
>>>> deal for me.
>>>>
>>> I don't how you do your editing, but in my case, 120 characters is 
>>> already so wide that I frequently have to shift my eyes horizontally 
>>> to read the whole line, and it doesn't fit on my editor window (with 
>>> 1600x1200 screen size).
>>> I expect many other developers to have even smaller screens, so I 
>>> figured 100 characters would be a reasonable suggestion.
>>> But like you said, it's just a recommendation.
>>
>> I find that long lines are typically exception messages or methods 
>> with long args.  In the vast majority of cases, the details are not 
>> important.
>
>
> Just whipped up a little line checker thingy to see what we're doing now.
>
> 80   = 91.19165439539742
> 100  = 5.488681894455669
> 120  = 2.004212184514229
> 140  = 0.805195027567549
> 141+ = 0.5102564980651347
>
> Seems we trail off after 120 lines.  I'd be fine with a soft 
> recommendation of 120 lines for non "signatures" (i.e. method 
> signatures, constructor signatures).  But please no code with strange 
> breaks like these:
>
> Example #1
>             this.serviceConstructor = 
> FastClass.create(this.enhancedServiceClass).getConstructor(
>             SERVICE_CONSTRUCTOR_TYPES);
>
> Example #2
>                 throw (NamingException) new NamingException("Could not 
> load service interface class " + serviceInterfaceClassName).
>                         initCause(e);
>
> In both situations, I'd probably leave the line long.  But if you 
> really feel compelled to get under 120, something like this would be 
> better:
>
> Example #1
>                 String message = "Could not load service interface 
> class " + serviceInterfaceClassName;
>                 throw (NamingException) new 
> NamingException(message).initCause(e);
>
> Example #2
>             FastClass fastClass = 
> FastClass.create(this.enhancedServiceClass);
>             this.serviceConstructor = 
> fastClass.getConstructor(SERVICE_CONSTRUCTOR_TYPES);
>
>
> Try to break lines in ways that end in either a '{' or a ';'.  Try not 
> to break matching '(' and ')' unless it an inner-class.  Please don't 
> dig out the '+' to break lines unless you're spanning more than just 
> one or two lines.
>
>
> -David
>
>
Updated openejb_eclipse_formatter.xml to 120 char max line length. Can 
these files be hosted in the wiki as attachments of some sort?

Re: Code conventions

Posted by David Blevins <da...@visi.com>.
On Oct 30, 2008, at 1:34 PM, Dain Sundstrom wrote:

> On Oct 30, 2008, at 1:09 PM, Alex Grönholm wrote:
>
>> Dain Sundstrom kirjoitti:
>>> On Oct 29, 2008, at 10:44 AM, Alex Grönholm wrote:
>>>
>>>> Formatter:
>>>> - max line width is 100 characters
>>>
>>> I prefer 120, but as long as it is only suggestion it isn't a big  
>>> deal for me.
>>>
>> I don't how you do your editing, but in my case, 120 characters is  
>> already so wide that I frequently have to shift my eyes  
>> horizontally to read the whole line, and it doesn't fit on my  
>> editor window (with 1600x1200 screen size).
>> I expect many other developers to have even smaller screens, so I  
>> figured 100 characters would be a reasonable suggestion.
>> But like you said, it's just a recommendation.
>
> I find that long lines are typically exception messages or methods  
> with long args.  In the vast majority of cases, the details are not  
> important.


Just whipped up a little line checker thingy to see what we're doing  
now.

80   = 91.19165439539742
100  = 5.488681894455669
120  = 2.004212184514229
140  = 0.805195027567549
141+ = 0.5102564980651347

Seems we trail off after 120 lines.  I'd be fine with a soft  
recommendation of 120 lines for non "signatures" (i.e. method  
signatures, constructor signatures).  But please no code with strange  
breaks like these:

Example #1
             this.serviceConstructor =  
FastClass.create(this.enhancedServiceClass).getConstructor(
             SERVICE_CONSTRUCTOR_TYPES);

Example #2
                 throw (NamingException) new NamingException("Could  
not load service interface class " + serviceInterfaceClassName).
                         initCause(e);

In both situations, I'd probably leave the line long.  But if you  
really feel compelled to get under 120, something like this would be  
better:

Example #1
                 String message = "Could not load service interface  
class " + serviceInterfaceClassName;
                 throw (NamingException) new  
NamingException(message).initCause(e);

Example #2
             FastClass fastClass =  
FastClass.create(this.enhancedServiceClass);
             this.serviceConstructor =  
fastClass.getConstructor(SERVICE_CONSTRUCTOR_TYPES);


Try to break lines in ways that end in either a '{' or a ';'.  Try not  
to break matching '(' and ')' unless it an inner-class.  Please don't  
dig out the '+' to break lines unless you're spanning more than just  
one or two lines.


-David



Re: Code conventions

Posted by Alex Grönholm <al...@nextday.fi>.
David Blevins kirjoitti:
>
> On Oct 30, 2008, at 2:24 PM, Alex Grönholm wrote:
>
>> Dain Sundstrom kirjoitti:
>>> On Oct 30, 2008, at 1:58 PM, Alex Grönholm wrote:
>>>>> if (debug) {
>>>>>   System.out.println("This is a one liner");
>>>>> }
>>>> I do not understand why the braces are necessary when the code 
>>>> block is only 1 line long. I used to do that years ago but over 
>>>> time I came to a decision to drop that practice since it didn't 
>>>> increase readability (to the contrary in fact) and it didn't 
>>>> particularly prevent any coding errors.
>>>
>>> Not for me.  I find it easy to make programming mistakes and it is 
>>> hard for me to read.
>>>
>> I on the other hand have strong issues with the one-liner you 
>> presented. Maybe we could agree to use braces for all control 
>> statements then? It takes up more space but at least neither of us 
>> would question its readability aspect.
>
> One liners are fine for short statements, otherwise I tend to use two 
> lines with braces.
>
> One example where I really like it:
>
>     protected Class resolveClass(ObjectStreamClass classDesc) throws 
> IOException, ClassNotFoundException {
>         try {
>             return Class.forName(classDesc.getName(), false, 
> getClassloader());
>         } catch (ClassNotFoundException e) {
>             String n = classDesc.getName();
>             if (n.equals("boolean")) return boolean.class;
>             if (n.equals("byte")) return byte.class;
>             if (n.equals("char")) return char.class;
>             if (n.equals("short")) return short.class;
>             if (n.equals("int")) return int.class;
>             if (n.equals("long")) return long.class;
>             if (n.equals("float")) return float.class;
>             if (n.equals("double")) return double.class;
>
>             throw e;
>         }
>     }
>
> If we can't agree than I'm fine with no recommendation as I don't have 
> a hard time reading any of those three formats.
>
> -David
>
I have updated openejb_eclipse_cleanup.xml and added 
http://paradoxx.dyndns.org/~demigod/openejb.importorder (Eclipse wanted 
to name it this way).

Re: Code conventions

Posted by David Blevins <da...@visi.com>.
On Oct 30, 2008, at 2:24 PM, Alex Grönholm wrote:

> Dain Sundstrom kirjoitti:
>> On Oct 30, 2008, at 1:58 PM, Alex Grönholm wrote:
>>>> if (debug) {
>>>>   System.out.println("This is a one liner");
>>>> }
>>> I do not understand why the braces are necessary when the code  
>>> block is only 1 line long. I used to do that years ago but over  
>>> time I came to a decision to drop that practice since it didn't  
>>> increase readability (to the contrary in fact) and it didn't  
>>> particularly prevent any coding errors.
>>
>> Not for me.  I find it easy to make programming mistakes and it is  
>> hard for me to read.
>>
> I on the other hand have strong issues with the one-liner you  
> presented. Maybe we could agree to use braces for all control  
> statements then? It takes up more space but at least neither of us  
> would question its readability aspect.

One liners are fine for short statements, otherwise I tend to use two  
lines with braces.

One example where I really like it:

     protected Class resolveClass(ObjectStreamClass classDesc) throws  
IOException, ClassNotFoundException {
         try {
             return Class.forName(classDesc.getName(), false,  
getClassloader());
         } catch (ClassNotFoundException e) {
             String n = classDesc.getName();
             if (n.equals("boolean")) return boolean.class;
             if (n.equals("byte")) return byte.class;
             if (n.equals("char")) return char.class;
             if (n.equals("short")) return short.class;
             if (n.equals("int")) return int.class;
             if (n.equals("long")) return long.class;
             if (n.equals("float")) return float.class;
             if (n.equals("double")) return double.class;

             throw e;
         }
     }

If we can't agree than I'm fine with no recommendation as I don't have  
a hard time reading any of those three formats.

-David


Re: Code conventions

Posted by Alex Grönholm <al...@nextday.fi>.
Dain Sundstrom kirjoitti:
> On Oct 30, 2008, at 1:58 PM, Alex Grönholm wrote:
>
>>>>>> Cleanup:
>>>>>> - always use braces with control statements (I actually disagree 
>>>>>> with this, and it's not even consistently used in existing code)
>>>>>
>>>>> One line if statement should be allowed.  For example,
>>>>>
>>>>> if (debug) System.out.println("This is a one liner");
>>>>>
>>>> I strongly recommend that the actual statement be put on a second 
>>>> line with an extra unit of indentation, like:
>>>> if (debug)
>>>>  System.out.println("This is a one liner");
>>>>
>>>> for readability. Makes it really clear it's a control statement there.
>>>
>>> I have strong feelings the other way, and more importantly, I have 
>>> strong feelings that any multiline control statement use braces, like:
>>>
>>> if (debug) {
>>>    System.out.println("This is a one liner");
>>> }
>> I do not understand why the braces are necessary when the code block 
>> is only 1 line long. I used to do that years ago but over time I came 
>> to a decision to drop that practice since it didn't increase 
>> readability (to the contrary in fact) and it didn't particularly 
>> prevent any coding errors.
>
> Not for me.  I find it easy to make programming mistakes and it is 
> hard for me to read.
>
I on the other hand have strong issues with the one-liner you presented. 
Maybe we could agree to use braces for all control statements then? It 
takes up more space but at least neither of us would question its 
readability aspect.
>>>>>> - removes unused imports and local variables
>>>>>
>>>>> Order and grouping of import should be specified. I use the 
>>>>> following:
>>>>>
>>>>> java.*
>>>>> javax.*
>>>>> <blank line>
>>>>> others
>>>>>
>>>>> I like this because it puts the important imports closest to the 
>>>>> code.
>>>>>
>>>> Works for me. I didn't think it was important so I didn't initially 
>>>> include that.
>>>
>>> It is important so that reformatting doesn't result in huge changes 
>>> in import blocks.
>> But this contradicts what you just said. If we include imports 
>> organization in the cleanup tasks and the imports are not already in 
>> the desired order, it will potentially cause big changes in import 
>> blocks then.
>
> In the short term.  More importantly when a programmer does reformat 
> the code it would like them to end up in a predefined and consistent 
> order.
>
I think the best I could come up with is:
java.*
<blank line>
javax.*
<blank line>
others

Would this be acceptable?
>>>>>> - removes "this" qualifier for non-static method/field accesses 
>>>>>> (where possible)
>>>>>> - adds missing @Override/@Deprecated annotations
>>>>>
>>>>> I don't like the @Override annotation in most cases.  The only 
>>>>> place I have used it is when doing complex subclassing of the SFSB 
>>>>> container in geronimo, where it helps catch interface changes.  In 
>>>>> openejb we typically don't do complex subclassing since it is so 
>>>>> brittle, so the @Override annotation just becomes annoying.
>>>> At least Eclipse inserts @Override automatically when you use the 
>>>> "Override/implement superclass methods" function from the Source 
>>>> menu. Even if you manually override something, two mouse clicks 
>>>> will insert the annotation. If all else fails, you could just use 
>>>> the cleanup function. I would guess IDEA has similar functionality 
>>>> to this, but I don't have it.
>>>
>>> IDEA can be set to do that also, and I would suggest that the 
>>> feature should be off for OpenEJB.  If a user wants to know 
>>> something is an override, they can simply look at the gutter notes 
>>> in the IDE.  Similarly, the auto generated javadocs in eclipse 
>>> should be turned off.
>> The @Override annotation is not used as a marker that the method 
>> overrides something, but as an additional safeguard that alerts the 
>> developer (by giving a compiler error) if the method no longer 
>> overrides a superclass method. This is far more useful with 
>> interfaces, but since @Override only works with interfaces in Java 6 
>> and we are targeting Java 5, this is admittely not as important.
>
> I understand how the annotation works, and I find it useful in the way 
> it was originally intended, to force the compiler to throw an 
> exception when a method is no-longer an override.  The common use as 
> documentation, brought about by the over zealous eclipse IDE, is 
> simply annoying and better served by gutter notes in an IDE.
>
I think we are in agreement that it's unnecessary as a means of 
documentation, but I would still like to keep it for safeguard purposes.
>>>>>> - removes unnecessary casts
>>>>>> - removes trailing whitespace
>>>>>> - corrects indentation
>>>>>>
>>>>>> Code templates:
>>>>>> - for newly create .java files, inserts the ASL comment to the top
>>>>>
>>>>> Also, in general, I dislike aligned text because it becomes a 
>>>>> burden to maintain over time.  There are places where it is nice 
>>>>> (specifically simple look tables), but I find it annoying in 
>>>>> javadoc comments and parameter declarations.
>>>>>
>>>> I don't think I suggested that anywhere, or maybe I misunderstand 
>>>> you. Could you please elaborate?
>>>
>>> You didn't, but I wanted to explicitly call that one out (that is 
>>> how much I dislike it).
>>>
>> One more thing about javadoc formatting -- what it does is this:
>> /**
>> * Method description
>> * @param foo string to indicate "foo"
>> */
>> is formatted to
>> /**
>> * Method description
>> *
>> * @param foo
>> *       string to indicate "foo"
>> */
>
> Yuck.
>
>> Helps readability in situations where you are editing the 
>> method/class itself so its javadocs aren't displayed by the IDE in a 
>> tooltip or similar.
>
> I don't know what you are talking about.  Intellij doesn't do that.
>
> -dain
Oh well, I'm not going to insist on that :P

Re: Code conventions

Posted by Dain Sundstrom <da...@iq80.com>.
On Oct 30, 2008, at 1:58 PM, Alex Grönholm wrote:

>>>>> Cleanup:
>>>>> - always use braces with control statements (I actually disagree  
>>>>> with this, and it's not even consistently used in existing code)
>>>>
>>>> One line if statement should be allowed.  For example,
>>>>
>>>> if (debug) System.out.println("This is a one liner");
>>>>
>>> I strongly recommend that the actual statement be put on a second  
>>> line with an extra unit of indentation, like:
>>> if (debug)
>>>  System.out.println("This is a one liner");
>>>
>>> for readability. Makes it really clear it's a control statement  
>>> there.
>>
>> I have strong feelings the other way, and more importantly, I have  
>> strong feelings that any multiline control statement use braces,  
>> like:
>>
>> if (debug) {
>>    System.out.println("This is a one liner");
>> }
> I do not understand why the braces are necessary when the code block  
> is only 1 line long. I used to do that years ago but over time I  
> came to a decision to drop that practice since it didn't increase  
> readability (to the contrary in fact) and it didn't particularly  
> prevent any coding errors.

Not for me.  I find it easy to make programming mistakes and it is  
hard for me to read.

>>>>> - removes unused imports and local variables
>>>>
>>>> Order and grouping of import should be specified. I use the  
>>>> following:
>>>>
>>>> java.*
>>>> javax.*
>>>> <blank line>
>>>> others
>>>>
>>>> I like this because it puts the important imports closest to the  
>>>> code.
>>>>
>>> Works for me. I didn't think it was important so I didn't  
>>> initially include that.
>>
>> It is important so that reformatting doesn't result in huge changes  
>> in import blocks.
> But this contradicts what you just said. If we include imports  
> organization in the cleanup tasks and the imports are not already in  
> the desired order, it will potentially cause big changes in import  
> blocks then.

In the short term.  More importantly when a programmer does reformat  
the code it would like them to end up in a predefined and consistent  
order.

>>>>> - removes "this" qualifier for non-static method/field accesses  
>>>>> (where possible)
>>>>> - adds missing @Override/@Deprecated annotations
>>>>
>>>> I don't like the @Override annotation in most cases.  The only  
>>>> place I have used it is when doing complex subclassing of the  
>>>> SFSB container in geronimo, where it helps catch interface  
>>>> changes.  In openejb we typically don't do complex subclassing  
>>>> since it is so brittle, so the @Override annotation just becomes  
>>>> annoying.
>>> At least Eclipse inserts @Override automatically when you use the  
>>> "Override/implement superclass methods" function from the Source  
>>> menu. Even if you manually override something, two mouse clicks  
>>> will insert the annotation. If all else fails, you could just use  
>>> the cleanup function. I would guess IDEA has similar functionality  
>>> to this, but I don't have it.
>>
>> IDEA can be set to do that also, and I would suggest that the  
>> feature should be off for OpenEJB.  If a user wants to know  
>> something is an override, they can simply look at the gutter notes  
>> in the IDE.  Similarly, the auto generated javadocs in eclipse  
>> should be turned off.
> The @Override annotation is not used as a marker that the method  
> overrides something, but as an additional safeguard that alerts the  
> developer (by giving a compiler error) if the method no longer  
> overrides a superclass method. This is far more useful with  
> interfaces, but since @Override only works with interfaces in Java 6  
> and we are targeting Java 5, this is admittely not as important.

I understand how the annotation works, and I find it useful in the way  
it was originally intended, to force the compiler to throw an  
exception when a method is no-longer an override.  The common use as  
documentation, brought about by the over zealous eclipse IDE, is  
simply annoying and better served by gutter notes in an IDE.

>>>>> - removes unnecessary casts
>>>>> - removes trailing whitespace
>>>>> - corrects indentation
>>>>>
>>>>> Code templates:
>>>>> - for newly create .java files, inserts the ASL comment to the top
>>>>
>>>> Also, in general, I dislike aligned text because it becomes a  
>>>> burden to maintain over time.  There are places where it is nice  
>>>> (specifically simple look tables), but I find it annoying in  
>>>> javadoc comments and parameter declarations.
>>>>
>>> I don't think I suggested that anywhere, or maybe I misunderstand  
>>> you. Could you please elaborate?
>>
>> You didn't, but I wanted to explicitly call that one out (that is  
>> how much I dislike it).
>>
> One more thing about javadoc formatting -- what it does is this:
> /**
> * Method description
> * @param foo string to indicate "foo"
> */
> is formatted to
> /**
> * Method description
> *
> * @param foo
> *       string to indicate "foo"
> */

Yuck.

> Helps readability in situations where you are editing the method/ 
> class itself so its javadocs aren't displayed by the IDE in a  
> tooltip or similar.

I don't know what you are talking about.  Intellij doesn't do that.

-dain

Re: Code conventions

Posted by Alex Grönholm <al...@nextday.fi>.
Dain Sundstrom kirjoitti:
> On Oct 30, 2008, at 1:09 PM, Alex Grönholm wrote:
>
>> Dain Sundstrom kirjoitti:
>>> On Oct 29, 2008, at 10:44 AM, Alex Grönholm wrote:
>>>
>>>> Formatter:
>>>> - max line width is 100 characters
>>>
>>> I prefer 120, but as long as it is only suggestion it isn't a big 
>>> deal for me.
>>>
>> I don't how you do your editing, but in my case, 120 characters is 
>> already so wide that I frequently have to shift my eyes horizontally 
>> to read the whole line, and it doesn't fit on my editor window (with 
>> 1600x1200 screen size).
>> I expect many other developers to have even smaller screens, so I 
>> figured 100 characters would be a reasonable suggestion.
>> But like you said, it's just a recommendation.
>
> I find that long lines are typically exception messages or methods 
> with long args.  In the vast majority of cases, the details are not 
> important.
>
>>>> Cleanup:
>>>> - always use braces with control statements (I actually disagree 
>>>> with this, and it's not even consistently used in existing code)
>>>
>>> One line if statement should be allowed.  For example,
>>>
>>> if (debug) System.out.println("This is a one liner");
>>>
>> I strongly recommend that the actual statement be put on a second 
>> line with an extra unit of indentation, like:
>> if (debug)
>>   System.out.println("This is a one liner");
>>
>> for readability. Makes it really clear it's a control statement there.
>
> I have strong feelings the other way, and more importantly, I have 
> strong feelings that any multiline control statement use braces, like:
>
> if (debug) {
>     System.out.println("This is a one liner");
> }
I do not understand why the braces are necessary when the code block is 
only 1 line long. I used to do that years ago but over time I came to a 
decision to drop that practice since it didn't increase readability (to 
the contrary in fact) and it didn't particularly prevent any coding errors.
>
>>>> - removes unused imports and local variables
>>>
>>> Order and grouping of import should be specified. I use the following:
>>>
>>> java.*
>>> javax.*
>>> <blank line>
>>> others
>>>
>>> I like this because it puts the important imports closest to the code.
>>>
>> Works for me. I didn't think it was important so I didn't initially 
>> include that.
>
> It is important so that reformatting doesn't result in huge changes in 
> import blocks.
But this contradicts what you just said. If we include imports 
organization in the cleanup tasks and the imports are not already in the 
desired order, it will potentially cause big changes in import blocks then.
>
>
>>>> - removes "this" qualifier for non-static method/field accesses 
>>>> (where possible)
>>>> - adds missing @Override/@Deprecated annotations
>>>
>>> I don't like the @Override annotation in most cases.  The only place 
>>> I have used it is when doing complex subclassing of the SFSB 
>>> container in geronimo, where it helps catch interface changes.  In 
>>> openejb we typically don't do complex subclassing since it is so 
>>> brittle, so the @Override annotation just becomes annoying.
>> At least Eclipse inserts @Override automatically when you use the 
>> "Override/implement superclass methods" function from the Source 
>> menu. Even if you manually override something, two mouse clicks will 
>> insert the annotation. If all else fails, you could just use the 
>> cleanup function. I would guess IDEA has similar functionality to 
>> this, but I don't have it.
>
> IDEA can be set to do that also, and I would suggest that the feature 
> should be off for OpenEJB.  If a user wants to know something is an 
> override, they can simply look at the gutter notes in the IDE.  
> Similarly, the auto generated javadocs in eclipse should be turned off.
The @Override annotation is not used as a marker that the method 
overrides something, but as an additional safeguard that alerts the 
developer (by giving a compiler error) if the method no longer overrides 
a superclass method. This is far more useful with interfaces, but since 
@Override only works with interfaces in Java 6 and we are targeting Java 
5, this is admittely not as important.
>
>>>> - removes unnecessary casts
>>>> - removes trailing whitespace
>>>> - corrects indentation
>>>>
>>>> Code templates:
>>>> - for newly create .java files, inserts the ASL comment to the top
>>>
>>> Also, in general, I dislike aligned text because it becomes a burden 
>>> to maintain over time.  There are places where it is nice 
>>> (specifically simple look tables), but I find it annoying in javadoc 
>>> comments and parameter declarations.
>>>
>> I don't think I suggested that anywhere, or maybe I misunderstand 
>> you. Could you please elaborate?
>
> You didn't, but I wanted to explicitly call that one out (that is how 
> much I dislike it).
>
One more thing about javadoc formatting -- what it does is this:
/**
 * Method description
 * @param foo string to indicate "foo"
 */
is formatted to
/**
 * Method description
 *
 * @param foo
 *       string to indicate "foo"
 */

Helps readability in situations where you are editing the method/class 
itself so its javadocs aren't displayed by the IDE in a tooltip or similar.
> -dain
>


Re: Code conventions

Posted by Dain Sundstrom <da...@iq80.com>.
On Oct 30, 2008, at 1:09 PM, Alex Grönholm wrote:

> Dain Sundstrom kirjoitti:
>> On Oct 29, 2008, at 10:44 AM, Alex Grönholm wrote:
>>
>>> Formatter:
>>> - max line width is 100 characters
>>
>> I prefer 120, but as long as it is only suggestion it isn't a big  
>> deal for me.
>>
> I don't how you do your editing, but in my case, 120 characters is  
> already so wide that I frequently have to shift my eyes horizontally  
> to read the whole line, and it doesn't fit on my editor window (with  
> 1600x1200 screen size).
> I expect many other developers to have even smaller screens, so I  
> figured 100 characters would be a reasonable suggestion.
> But like you said, it's just a recommendation.

I find that long lines are typically exception messages or methods  
with long args.  In the vast majority of cases, the details are not  
important.

>>> Cleanup:
>>> - always use braces with control statements (I actually disagree  
>>> with this, and it's not even consistently used in existing code)
>>
>> One line if statement should be allowed.  For example,
>>
>> if (debug) System.out.println("This is a one liner");
>>
> I strongly recommend that the actual statement be put on a second  
> line with an extra unit of indentation, like:
> if (debug)
>   System.out.println("This is a one liner");
>
> for readability. Makes it really clear it's a control statement there.

I have strong feelings the other way, and more importantly, I have  
strong feelings that any multiline control statement use braces, like:

if (debug) {
     System.out.println("This is a one liner");
}

>>> - removes unused imports and local variables
>>
>> Order and grouping of import should be specified. I use the  
>> following:
>>
>> java.*
>> javax.*
>> <blank line>
>> others
>>
>> I like this because it puts the important imports closest to the  
>> code.
>>
> Works for me. I didn't think it was important so I didn't initially  
> include that.

It is important so that reformatting doesn't result in huge changes in  
import blocks.

>>> - removes "this" qualifier for non-static method/field accesses  
>>> (where possible)
>>> - adds missing @Override/@Deprecated annotations
>>
>> I don't like the @Override annotation in most cases.  The only  
>> place I have used it is when doing complex subclassing of the SFSB  
>> container in geronimo, where it helps catch interface changes.  In  
>> openejb we typically don't do complex subclassing since it is so  
>> brittle, so the @Override annotation just becomes annoying.
> At least Eclipse inserts @Override automatically when you use the  
> "Override/implement superclass methods" function from the Source  
> menu. Even if you manually override something, two mouse clicks will  
> insert the annotation. If all else fails, you could just use the  
> cleanup function. I would guess IDEA has similar functionality to  
> this, but I don't have it.

IDEA can be set to do that also, and I would suggest that the feature  
should be off for OpenEJB.  If a user wants to know something is an  
override, they can simply look at the gutter notes in the IDE.   
Similarly, the auto generated javadocs in eclipse should be turned off.

>>> - removes unnecessary casts
>>> - removes trailing whitespace
>>> - corrects indentation
>>>
>>> Code templates:
>>> - for newly create .java files, inserts the ASL comment to the top
>>
>> Also, in general, I dislike aligned text because it becomes a  
>> burden to maintain over time.  There are places where it is nice  
>> (specifically simple look tables), but I find it annoying in  
>> javadoc comments and parameter declarations.
>>
> I don't think I suggested that anywhere, or maybe I misunderstand  
> you. Could you please elaborate?

You didn't, but I wanted to explicitly call that one out (that is how  
much I dislike it).

-dain


Re: Code conventions

Posted by Alex Grönholm <al...@nextday.fi>.
Dain Sundstrom kirjoitti:
> On Oct 29, 2008, at 10:44 AM, Alex Grönholm wrote:
>
>> Formatter:
>> - max line width is 100 characters
>
> I prefer 120, but as long as it is only suggestion it isn't a big deal 
> for me.
>
I don't how you do your editing, but in my case, 120 characters is 
already so wide that I frequently have to shift my eyes horizontally to 
read the whole line, and it doesn't fit on my editor window (with 
1600x1200 screen size).
I expect many other developers to have even smaller screens, so I 
figured 100 characters would be a reasonable suggestion.
But like you said, it's just a recommendation.
>> - indentation is 4 spaces
>
> Would XML be 4 spaces also?  I don't care if it is 2 or 4.
>
I meant 4 spaces for Java source files. For XML it could be either, I 
don't really care.
>> - javadocs are formatted
>
> What does that mean?
>
>> Cleanup:
>> - always use braces with control statements (I actually disagree with 
>> this, and it's not even consistently used in existing code)
>
> One line if statement should be allowed.  For example,
>
> if (debug) System.out.println("This is a one liner");
>
I strongly recommend that the actual statement be put on a second line 
with an extra unit of indentation, like:
if (debug)
    System.out.println("This is a one liner");

for readability. Makes it really clear it's a control statement there.
>> - removes unused imports and local variables
>
> Order and grouping of import should be specified. I use the following:
>
> java.*
> javax.*
> <blank line>
> others
>
> I like this because it puts the important imports closest to the code.
>
Works for me. I didn't think it was important so I didn't initially 
include that.
>> - removes "this" qualifier for non-static method/field accesses 
>> (where possible)
>> - adds missing @Override/@Deprecated annotations
>
> I don't like the @Override annotation in most cases.  The only place I 
> have used it is when doing complex subclassing of the SFSB container 
> in geronimo, where it helps catch interface changes.  In openejb we 
> typically don't do complex subclassing since it is so brittle, so the 
> @Override annotation just becomes annoying.
At least Eclipse inserts @Override automatically when you use the 
"Override/implement superclass methods" function from the Source menu. 
Even if you manually override something, two mouse clicks will insert 
the annotation. If all else fails, you could just use the cleanup 
function. I would guess IDEA has similar functionality to this, but I 
don't have it.
>
>
>> - removes unnecessary casts
>> - removes trailing whitespace
>> - corrects indentation
>>
>> Code templates:
>> - for newly create .java files, inserts the ASL comment to the top
>
> Also, in general, I dislike aligned text because it becomes a burden 
> to maintain over time.  There are places where it is nice 
> (specifically simple look tables), but I find it annoying in javadoc 
> comments and parameter declarations.
>
I don't think I suggested that anywhere, or maybe I misunderstand you. 
Could you please elaborate?
>
>


Re: Code conventions

Posted by Dain Sundstrom <da...@iq80.com>.
On Oct 29, 2008, at 10:44 AM, Alex Grönholm wrote:

> Formatter:
> - max line width is 100 characters

I prefer 120, but as long as it is only suggestion it isn't a big deal  
for me.

> - indentation is 4 spaces

Would XML be 4 spaces also?  I don't care if it is 2 or 4.

> - javadocs are formatted

What does that mean?

> Cleanup:
> - always use braces with control statements (I actually disagree  
> with this, and it's not even consistently used in existing code)

One line if statement should be allowed.  For example,

if (debug) System.out.println("This is a one liner");

> - removes unused imports and local variables

Order and grouping of import should be specified. I use the following:

java.*
javax.*
<blank line>
others

I like this because it puts the important imports closest to the code.

> - removes "this" qualifier for non-static method/field accesses  
> (where possible)
> - adds missing @Override/@Deprecated annotations

I don't like the @Override annotation in most cases.  The only place I  
have used it is when doing complex subclassing of the SFSB container  
in geronimo, where it helps catch interface changes.  In openejb we  
typically don't do complex subclassing since it is so brittle, so the  
@Override annotation just becomes annoying.

> - removes unnecessary casts
> - removes trailing whitespace
> - corrects indentation
>
> Code templates:
> - for newly create .java files, inserts the ASL comment to the top

Also, in general, I dislike aligned text because it becomes a burden  
to maintain over time.  There are places where it is nice  
(specifically simple look tables), but I find it annoying in javadoc  
comments and parameter declarations.