You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Mike Bowler <mb...@GargoyleSoftware.com> on 2003/01/27 17:38:57 UTC
[HttpClient] Proposed style change
I am going through the code and changing it to conform to the style as
per checkstyle. There are two places that I think we may want to be a
little more lenient than the defaults and so I'm putting them here for
discussion.
1) Line length of 80
The number 80 originally came from the days when printers could only
print 80 columns. These days, that number doesn't make nearly as much
sense. While I think we still want to have a maximum length, I believe
that 80 is too short.
In most of the places that we are exceeding the limit, we are just over
by a bit (between 80 and 90). I propose that we change the max length
to 100. This is still short enough that nobody should have to scroll to
see the source but long enough that we aren't artificially breaking lines.
2) Instance variable names
Some of the code uses leading underscores for variable names but this
isn't allowed by checkstyle. The pattern it checks for is
^[a-z][a-zA-Z0-9]*$
I find significant benefit to being able to quickly distinguish instance
variables from local variables and the leading underscore lets me do
that easily. (I personally prefer trailing underscores to leading ones
but I can live with either)
I propose that we change the pattern for instance variables to
^_?[a-z][a-zA-Z0-9]*$ so that we will allow leading underscores but will
not insist on it.
Comments?
--
Mike Bowler
Principal, Gargoyle Software Inc.
Voice: (416) 822-0973 | Email : mbowler@GargoyleSoftware.com
Fax : (416) 822-0975 | Website: http://www.GargoyleSoftware.com
Re: [HttpClient] Proposed style change
Posted by Jeffrey Dever <js...@sympatico.ca>.
My proposals for changes to the default sun style rules:
checkstyle.maxlinelen=100
I agree with Mike and Simon that the line length should be longer. 100
is fairly standard in other projects. (comment blocks should still be
80 however it is not enforceable with checkstyle). Martin brought up a
good point about side by side diff needing a narrow format, but I think
that artifically braking lines needlessly does more dammage than having
to scroll a little bit in a diff. Besides, if a diff user is unhappy
with line length, they can use a higher resolution, but if somone doe
not like many broken lines, they have no recourse.
checkstyle.pattern.publicmember=^[a-z][a-zA-Z0-9]*$
The default is ^f[A-Z][a-zA-Z0-9]*$ which is apparently for EJB but is
just plain stupid in the HttpClient context.
checkstyle.pattern.package=^[a-z]+(\.[a-z]*)*$
The default is ^[a-z]+(\.[a-zA-Z_][a-zA-Z_0-9]*)*$ which allows caps,
underscores and numbers. HttpClient does not use these symbols, and we
shouldn't start using them.
checkstyle.header.file=license.regexp
checkstyle.header.regexp=true
http://cvs.apache.org/viewcvs/jakarta-commons/httpclient/license.regexp?rev=HEAD&content-type=text/vnd.viewcvs-markup
This is to enforce that all source files have the commons license, the
usual CVS keywords, and a current date that makes sense. (this is
already in CVS)
checkstyle.ignore.maxlinelen=Header:
This is to prevent any warning for the expansion of the $Header$ cvs
keyword which is always very long. I wish I had a better regexp for
this that 1) works and 2) does not itself get expanded as a keyword when
the checkstyle.properties file is commited. (this is already in CVS)
checkstyle.tab.width=4
This does not really matter for us as tabs are turned off, but the world
would be a better place if the standard size for a tab was 4 spaces.
Jandalf.
Mike Bowler wrote:
> I am going through the code and changing it to conform to the style as
> per checkstyle. There are two places that I think we may want to be a
> little more lenient than the defaults and so I'm putting them here for
> discussion.
>
> 1) Line length of 80
>
> The number 80 originally came from the days when printers could only
> print 80 columns. These days, that number doesn't make nearly as much
> sense. While I think we still want to have a maximum length, I
> believe that 80 is too short.
>
> In most of the places that we are exceeding the limit, we are just
> over by a bit (between 80 and 90). I propose that we change the max
> length to 100. This is still short enough that nobody should have to
> scroll to see the source but long enough that we aren't artificially
> breaking lines.
>
> 2) Instance variable names
>
> Some of the code uses leading underscores for variable names but this
> isn't allowed by checkstyle. The pattern it checks for is
> ^[a-z][a-zA-Z0-9]*$
>
> I find significant benefit to being able to quickly distinguish
> instance variables from local variables and the leading underscore
> lets me do that easily. (I personally prefer trailing underscores to
> leading ones but I can live with either)
>
> I propose that we change the pattern for instance variables to
> ^_?[a-z][a-zA-Z0-9]*$ so that we will allow leading underscores but
> will not insist on it.
>
> Comments?
>
Re: [HttpClient] Proposed style change
Posted by Sung-Gu <je...@apache.org>.
Personally,
> 1) Line length of 80
+1
> 2) Instance variable names
> ^_?[a-z][a-zA-Z0-9]
+0
Sung-Gu
Re: [HttpClient] Proposed style change
Posted by Jeffrey Dever <js...@sympatico.ca>.
>
>
>>I propose that we change the pattern for instance variables to
>>^_?[a-z][a-zA-Z0-9]*$ so that we will allow leading underscores but will
>>not insist on it.
>>
>>
>
>Uuuugh, please no! Besides being ugly, it's worth sticking to the Sun
>coding
>style (as is the default with checkstyle). That way, anyone who has done any
>Java will be immediately familiar with it.
>
>
>
I'd agree with Simon on this. Leading (and trailing) underscores are
ugly (just look a Python!). If its too difficult to determine if a
variable is a member or a local, then the method is probablly too big.
And you can always prefix this to a member for clarity.
Re: [HttpClient] Proposed style change
Posted by Simon Roberts <si...@fifthweb.net>.
> 1) Line length of 80
> In most of the places that we are exceeding the limit, we are just over
> by a bit (between 80 and 90). I propose that we change the max length
> to 100. This is still short enough that nobody should have to scroll to
> see the source but long enough that we aren't artificially breaking lines.
Sure. We use 100 for checkstyle here too.
> 2) Instance variable names
> Some of the code uses leading underscores for variable names but this
> isn't allowed by checkstyle. The pattern it checks for is
> ^[a-z][a-zA-Z0-9]*$
> I find significant benefit to being able to quickly distinguish instance
> variables from local variables and the leading underscore lets me do
> that easily. (I personally prefer trailing underscores to leading ones
> but I can live with either)
>
> I propose that we change the pattern for instance variables to
> ^_?[a-z][a-zA-Z0-9]*$ so that we will allow leading underscores but will
> not insist on it.
Uuuugh, please no! Besides being ugly, it's worth sticking to the Sun
coding
style (as is the default with checkstyle). That way, anyone who has done any
Java will be immediately familiar with it.
$0.02 Simon
Re: [HttpClient] Proposed style change
Posted by Jeffrey Dever <js...@sympatico.ca>.
Ah ha! I knew that this was going to come up, and I'm somewhat prepared.
Both of your suggestions do have precedent in other Jakarta projects. I
am open to the idea of having a coding standard specific to HttpClient.
It is our right to have our own coding standard, if we so choose.
Perhaps after some research, and discussion, we could propose a vote on
a number of vaiations to the coding guidelines. The only restriction
that I would put is that every particular style variation proposed as a
concrete change to the checkstyle.properties file that is now in our
repository.
I also expect some input from the commons proper guys.
Some other resources to consider:
http://jakarta.apache.org/site/faqs.html
http://jakarta.apache.org/commons/patches.html
http://jakarta.apache.org/cactus/coding_conventions.html
http://jakarta.apache.org/avalon/code-standards.html
http://jakarta.apache.org/velocity/code-standards.html
http://jakarta.apache.org/jetspeed/site/code-standards.html
http://jakarta.apache.org/commons/sandbox/net/code-standards.html
http://jakarta.apache.org/commons/latka/developers-guide.html
So talk about your suggestions, and post some precise proposals and I'll
get them all togehter at the end of the week and we'll have a vote. (we
should hold off on any style specific patches untill this is sorted out)
-jsd
Mike Bowler wrote:
> I am going through the code and changing it to conform to the style as
> per checkstyle. There are two places that I think we may want to be a
> little more lenient than the defaults and so I'm putting them here for
> discussion.
>
> 1) Line length of 80
>
> The number 80 originally came from the days when printers could only
> print 80 columns. These days, that number doesn't make nearly as much
> sense. While I think we still want to have a maximum length, I
> believe that 80 is too short.
>
> In most of the places that we are exceeding the limit, we are just
> over by a bit (between 80 and 90). I propose that we change the max
> length to 100. This is still short enough that nobody should have to
> scroll to see the source but long enough that we aren't artificially
> breaking lines.
>
> 2) Instance variable names
>
> Some of the code uses leading underscores for variable names but this
> isn't allowed by checkstyle. The pattern it checks for is
> ^[a-z][a-zA-Z0-9]*$
>
> I find significant benefit to being able to quickly distinguish
> instance variables from local variables and the leading underscore
> lets me do that easily. (I personally prefer trailing underscores to
> leading ones but I can live with either)
>
> I propose that we change the pattern for instance variables to
> ^_?[a-z][a-zA-Z0-9]*$ so that we will allow leading underscores but
> will not insist on it.
>
> Comments?
>