Writing Readable Code

[Kevan Miller <ke...@gmail.com>]

During the recent discussions regarding the Geronimo Development  
process, several people expressed some concern about moving away from  
RTC.  The biggest issue seemed to be that RTC insured multiple people  
reviewed new code. Having reviewed the code, the reviewers now  
understood and would be able to support the code (i.e. fix bugs).

This is certainly a valid concern. However, even though we're now  
following CTR, we all need to be making a concerted effort to provide  
the same level of review as commits are made.

No matter what process we're following, IMO, the best way to insure  
that people are reading *and* understanding your code is to write  
code that is easy to read and understand. This does not mean writing  
simple code. It simply means keeping the reader in mind and trying to  
make their job easier.

The single, most important thing, in my mind, is to provide clear and  
insightful comments to assist the reader. These don't need to be  
verbose tomes. They don't need to state the obvious. However, any  
assistance you can provide the reader is helpful. Describe the  
processing flow that methods are being invoked. What are the  
threading assumptions? Identify subtleties that a reader might not be  
aware of. Who are the potential callers? etc...

In case anyone is wondering -- I think we've been lacking in this  
department. I'd like to see simple comment guidelines incorporated  
into the Documentation Guidelines for our CTR process.

In my opinion, failure to appropriately comment new code is cause for  
a commit to be vetoed. I doubt that this will happen often. I expect  
that in most instances these issues can be resolved appropriately  
through simple discussion. Some basic rules-of-thumb are likely to  
help resolve any issues.

What do others think?

Specific ideas on comment guidelines? Javadoc-style comments for APIs/ 
SPIs, etc? What types of comments should be expected?

--kevan
  

Re: Writing Readable Code

[Jeff Genender <jg...@apache.org>]

Kevan Miller wrote:

> In my experience, the probability of good comments being added to code
> post the initial commit is extremely low. If you think it would be good
> to document more, how do you see that happening?

Be a nudge ;-)  Your comments to Gianny about having a bit more doc for
the clustering component were great.  It wasn't a gate, but you are
expressing a need for a bit more javadoc.  I think this is a very
abstract/gray area for how/what/where/when.  In cases where appropriate,
just speak up.  We all will eventually get it...since people who are the
largest violators will hopefully want to go with the flow (and not keep
getting nudged). :-)

> 
> --kevan
> 
> 
> 

Re: Writing Readable Code

[Kevan Miller <ke...@gmail.com>]

On Sep 17, 2006, at 4:45 PM, Jeff Genender wrote:

> I am not supportive of forcing javadoc, and I would not like to make
> that a reason for veto.  If folks aren't following guidelines to good
> coding practices and the code is illegible, then I think we can help
> mentor following generally accepted practices.  I have read a lot of
> code and for 99% of the time, the code is understandable enough.

I'm hesitant to be too dogmatic on any rules. Personally, I think  
javadoc for API/SPI classes/interfaces is reasonable and desirable.  
I'd give people more leeway on internal apis and implementation (the  
majority of our code).

Sure, most code can be read. However, often times you can read it  
more quickly with a little assist. Also, sometimes reading code means  
unknowingly reading "bugs". Also, it can be difficult to understand  
the context in which code is invoked. For example, understanding the  
threading context of a method invocation can be difficult. When a  
method is a part of an overall larger picture, pulling that context  
together can be extremely useful to the reader.

>
> I think if CTR is the order, then people who make fairly large commits
> should engage in some thorough discussion of what they are doing on  
> the
> lists.  Javadoc should not be a substitute for this a s javadoc is not
> engaging the community, which is most important.

Absolutely agreed that comments are no substitute for communicating  
changes to the community. However, they can also enhance that  
community communication.

>
> OTOH, it would be nice if we did document a little bit more.  But I
> would not like to see it as a gate to getting code in.

In my experience, the probability of good comments being added to  
code post the initial commit is extremely low. If you think it would  
be good to document more, how do you see that happening?

--kevan

Re: Writing Readable Code

[Jeff Genender <jg...@apache.org>]

I am not supportive of forcing javadoc, and I would not like to make
that a reason for veto.  If folks aren't following guidelines to good
coding practices and the code is illegible, then I think we can help
mentor following generally accepted practices.  I have read a lot of
code and for 99% of the time, the code is understandable enough.

I think if CTR is the order, then people who make fairly large commits
should engage in some thorough discussion of what they are doing on the
lists.  Javadoc should not be a substitute for this a s javadoc is not
engaging the community, which is most important.

OTOH, it would be nice if we did document a little bit more.  But I
would not like to see it as a gate to getting code in.

Jeff

Kevan Miller wrote:
> During the recent discussions regarding the Geronimo Development
> process, several people expressed some concern about moving away from
> RTC.  The biggest issue seemed to be that RTC insured multiple people
> reviewed new code. Having reviewed the code, the reviewers now
> understood and would be able to support the code (i.e. fix bugs).
> 
> This is certainly a valid concern. However, even though we're now
> following CTR, we all need to be making a concerted effort to provide
> the same level of review as commits are made.
> 
> No matter what process we're following, IMO, the best way to insure that
> people are reading *and* understanding your code is to write code that
> is easy to read and understand. This does not mean writing simple code.
> It simply means keeping the reader in mind and trying to make their job
> easier.
> 
> The single, most important thing, in my mind, is to provide clear and
> insightful comments to assist the reader. These don't need to be verbose
> tomes. They don't need to state the obvious. However, any assistance you
> can provide the reader is helpful. Describe the processing flow that
> methods are being invoked. What are the threading assumptions? Identify
> subtleties that a reader might not be aware of. Who are the potential
> callers? etc...
> 
> In case anyone is wondering -- I think we've been lacking in this
> department. I'd like to see simple comment guidelines incorporated into
> the Documentation Guidelines for our CTR process.
> 
> In my opinion, failure to appropriately comment new code is cause for a
> commit to be vetoed. I doubt that this will happen often. I expect that
> in most instances these issues can be resolved appropriately through
> simple discussion. Some basic rules-of-thumb are likely to help resolve
> any issues.
> 
> What do others think?
> 
> Specific ideas on comment guidelines? Javadoc-style comments for
> APIs/SPIs, etc? What types of comments should be expected?
> 
> --kevan
>  

Re: Writing Readable Code

[Paul McMahan <pa...@gmail.com>]

On 9/18/06, Kevan Miller <ke...@gmail.com> wrote:
>
> I'd certainly want to see issues resolved via discussion. We can
> certainly try that, initially.
>
> However, I think we should acknowledge that if the code isn't
> commented at commit (or very soon afterwards), the code will most
> likely never be commented...

Yup I concede on that point, and also find it disturbing.  So in some
cases I bet no one would object to a veto.

Paul

Re: Writing Readable Code

[Kevan Miller <ke...@gmail.com>]

On Sep 18, 2006, at 11:17 AM, Paul McMahan wrote:

> I agree that readability is a huge factor in code quality and is a key
> to its survival.  But I tend to disagree that a commit should be
> rejected based on this criteria alone. IMHO the low bar for committing
> to trunk should be that it compiles, does no harm, and has undergone
> adequate discussion within the dev community.  Providing adequate
> comments lies just outside that threshold as something that you should
> expect to be nagged heavily about but not necessarily vetoed.  Like
> you say, in most cases this will be resolved through simple
> discussion.

I'd certainly want to see issues resolved via discussion. We can  
certainly try that, initially.

However, I think we should acknowledge that if the code isn't  
commented at commit (or very soon afterwards), the code will most  
likely never be commented...

--kevan 

Re: Writing Readable Code

[Matt Hogstrom <ma...@hogstrom.org>]

On Sep 18, 2006, at 11:17 AM, Paul McMahan wrote:

> I agree that readability is a huge factor in code quality and is a key
> to its survival.  But I tend to disagree that a commit should be
> rejected based on this criteria alone. IMHO the low bar for committing
> to trunk should be that it compiles, does no harm, and has undergone
> adequate discussion within the dev community.

I also agree it shouldn't be a gate to committing the code.  But for  
the person who is looking at the code six months from now it might be  
nice to have part of that discussion on the dev list condensed into  
the code in the form of a comment.  At least that way people wouldn't  
be forced to mine for information about a module through Google.

I also think rules in this area will be impossible to enforce and  
will slow things down.  If there has to be discussion about something  
in e-mail from a commit then perhaps that is an indication that the  
code wasn't clear to begin with and might need a bit of clarification.

> Providing adequate
> comments lies just outside that threshold as something that you should
> expect to be nagged heavily about but not necessarily vetoed.  Like
> you say, in most cases this will be resolved through simple
> discussion.
>
> Best wishes,
> Paul

Re: Writing Readable Code

[Paul McMahan <pa...@gmail.com>]

I agree that readability is a huge factor in code quality and is a key
to its survival.  But I tend to disagree that a commit should be
rejected based on this criteria alone. IMHO the low bar for committing
to trunk should be that it compiles, does no harm, and has undergone
adequate discussion within the dev community.  Providing adequate
comments lies just outside that threshold as something that you should
expect to be nagged heavily about but not necessarily vetoed.  Like
you say, in most cases this will be resolved through simple
discussion.

Best wishes,
Paul


On 9/15/06, Kevan Miller <ke...@gmail.com> wrote:
> During the recent discussions regarding the Geronimo Development
> process, several people expressed some concern about moving away from
> RTC.  The biggest issue seemed to be that RTC insured multiple people
> reviewed new code. Having reviewed the code, the reviewers now
> understood and would be able to support the code (i.e. fix bugs).
>
> This is certainly a valid concern. However, even though we're now
> following CTR, we all need to be making a concerted effort to provide
> the same level of review as commits are made.
>
> No matter what process we're following, IMO, the best way to insure
> that people are reading *and* understanding your code is to write
> code that is easy to read and understand. This does not mean writing
> simple code. It simply means keeping the reader in mind and trying to
> make their job easier.
>
> The single, most important thing, in my mind, is to provide clear and
> insightful comments to assist the reader. These don't need to be
> verbose tomes. They don't need to state the obvious. However, any
> assistance you can provide the reader is helpful. Describe the
> processing flow that methods are being invoked. What are the
> threading assumptions? Identify subtleties that a reader might not be
> aware of. Who are the potential callers? etc...
>
> In case anyone is wondering -- I think we've been lacking in this
> department. I'd like to see simple comment guidelines incorporated
> into the Documentation Guidelines for our CTR process.
>
> In my opinion, failure to appropriately comment new code is cause for
> a commit to be vetoed. I doubt that this will happen often. I expect
> that in most instances these issues can be resolved appropriately
> through simple discussion. Some basic rules-of-thumb are likely to
> help resolve any issues.
>
> What do others think?
>
> Specific ideas on comment guidelines? Javadoc-style comments for APIs/
> SPIs, etc? What types of comments should be expected?
>
> --kevan
>
>

Re: Writing Readable Code

[Joe Bohn <jo...@earthlink.net>]

Jason Dillon wrote:
> Color comments to some hideous color so you can't miss them.  block  
> comments in code is a pain the ass when you need to comment out  
> something.  I personally never easily miss a // comment :-P, but then  
> again, I normally make comments in code using // multiline with NOTE:  
> or HACK: or TODO: which IDEA likes to add extra color too.

Sorry, I got a bit loose with my terminology.  Yes, I agree with // 
multi-line comment style and personally prefer that to /* ... */ style. 
  What I was more intending to avoid was the infamous 200 char code line 
followed by a comment on the same line which I sometimes miss when I'm 
just scanning code to get the general idea of what it's doing.

> 
> I think at the minimum that classes *must* all have javadoc...  forcing 
> each method to have javadoc is lame and will endup with lame  javadocs 
> that are meaningless... like getName() this method gets the  name, 
> setName() this method sets the name.  But, I am generally in  favor of 
> javadocs, and package docs.  Wish more of G had them... pity  that it 
> doesn't.

Yes, I agree with this too.  I was thinking of more interesting methods. 
  Getters, setters, and constructors are typically self-explanatory.

> 
> IMO what to doc and what not to doc just comes naturally to me... and  I 
> like to add them as it augments the readability of the code and the  
> beauty too... maybe I am a dork, but I like to javadoc my code and  then 
> go read the generated html... makes me feel good.
> 
> --jason
> 
> 
> On Sep 17, 2006, at 1:05 PM, Joe Bohn wrote:
> 
>> I completely agree.
>>
>> There are some areas of the code that contain virtually no  comments. 
>> Any comments are better than none.  Better still would be  some 
>> guidelines as you've suggested.
>>
>> At a minimum I think that each method should have javadoc style  
>> comments that actually explain the purpose of the method as opposed  
>> to simply echoing the obvious (param name/type definitions and  return 
>> types).  I think that descriptive comments highlighting the  purpose 
>> of the method are most helpful.  I also prefer block  comments at 
>> critical places in the code to describe the flow over  EOL comments 
>> which are easily missed.
>>
>> Joe
>>
>>
>> Kevan Miller wrote:
>>
>>> During the recent discussions regarding the Geronimo Development   
>>> process, several people expressed some concern about moving away  
>>> from  RTC.  The biggest issue seemed to be that RTC insured  multiple 
>>> people  reviewed new code. Having reviewed the code, the  reviewers 
>>> now  understood and would be able to support the code  (i.e. fix bugs).
>>> This is certainly a valid concern. However, even though we're now   
>>> following CTR, we all need to be making a concerted effort to  
>>> provide  the same level of review as commits are made.
>>> No matter what process we're following, IMO, the best way to  insure  
>>> that people are reading *and* understanding your code is  to write  
>>> code that is easy to read and understand. This does not  mean 
>>> writing  simple code. It simply means keeping the reader in  mind and 
>>> trying to  make their job easier.
>>> The single, most important thing, in my mind, is to provide clear  
>>> and  insightful comments to assist the reader. These don't need to  
>>> be  verbose tomes. They don't need to state the obvious. However,  
>>> any  assistance you can provide the reader is helpful. Describe  the  
>>> processing flow that methods are being invoked. What are the   
>>> threading assumptions? Identify subtleties that a reader might not  
>>> be  aware of. Who are the potential callers? etc...
>>> In case anyone is wondering -- I think we've been lacking in this   
>>> department. I'd like to see simple comment guidelines  incorporated  
>>> into the Documentation Guidelines for our CTR process.
>>> In my opinion, failure to appropriately comment new code is cause  
>>> for  a commit to be vetoed. I doubt that this will happen often. I  
>>> expect  that in most instances these issues can be resolved  
>>> appropriately  through simple discussion. Some basic rules-of- thumb 
>>> are likely to  help resolve any issues.
>>> What do others think?
>>> Specific ideas on comment guidelines? Javadoc-style comments for  
>>> APIs/ SPIs, etc? What types of comments should be expected?
>>> --kevan
>>>
> 
> 

Re: Writing Readable Code

[Jason Dillon <ja...@planet57.com>]

Color comments to some hideous color so you can't miss them.  block  
comments in code is a pain the ass when you need to comment out  
something.  I personally never easily miss a // comment :-P, but then  
again, I normally make comments in code using // multiline with NOTE:  
or HACK: or TODO: which IDEA likes to add extra color too.

I think at the minimum that classes *must* all have javadoc...  
forcing each method to have javadoc is lame and will endup with lame  
javadocs that are meaningless... like getName() this method gets the  
name, setName() this method sets the name.  But, I am generally in  
favor of javadocs, and package docs.  Wish more of G had them... pity  
that it doesn't.

IMO what to doc and what not to doc just comes naturally to me... and  
I like to add them as it augments the readability of the code and the  
beauty too... maybe I am a dork, but I like to javadoc my code and  
then go read the generated html... makes me feel good.

--jason


On Sep 17, 2006, at 1:05 PM, Joe Bohn wrote:

> I completely agree.
>
> There are some areas of the code that contain virtually no  
> comments. Any comments are better than none.  Better still would be  
> some guidelines as you've suggested.
>
> At a minimum I think that each method should have javadoc style  
> comments that actually explain the purpose of the method as opposed  
> to simply echoing the obvious (param name/type definitions and  
> return types).  I think that descriptive comments highlighting the  
> purpose of the method are most helpful.  I also prefer block  
> comments at critical places in the code to describe the flow over  
> EOL comments which are easily missed.
>
> Joe
>
>
> Kevan Miller wrote:
>> During the recent discussions regarding the Geronimo Development   
>> process, several people expressed some concern about moving away  
>> from  RTC.  The biggest issue seemed to be that RTC insured  
>> multiple people  reviewed new code. Having reviewed the code, the  
>> reviewers now  understood and would be able to support the code  
>> (i.e. fix bugs).
>> This is certainly a valid concern. However, even though we're now   
>> following CTR, we all need to be making a concerted effort to  
>> provide  the same level of review as commits are made.
>> No matter what process we're following, IMO, the best way to  
>> insure  that people are reading *and* understanding your code is  
>> to write  code that is easy to read and understand. This does not  
>> mean writing  simple code. It simply means keeping the reader in  
>> mind and trying to  make their job easier.
>> The single, most important thing, in my mind, is to provide clear  
>> and  insightful comments to assist the reader. These don't need to  
>> be  verbose tomes. They don't need to state the obvious. However,  
>> any  assistance you can provide the reader is helpful. Describe  
>> the  processing flow that methods are being invoked. What are the   
>> threading assumptions? Identify subtleties that a reader might not  
>> be  aware of. Who are the potential callers? etc...
>> In case anyone is wondering -- I think we've been lacking in this   
>> department. I'd like to see simple comment guidelines  
>> incorporated  into the Documentation Guidelines for our CTR process.
>> In my opinion, failure to appropriately comment new code is cause  
>> for  a commit to be vetoed. I doubt that this will happen often. I  
>> expect  that in most instances these issues can be resolved  
>> appropriately  through simple discussion. Some basic rules-of- 
>> thumb are likely to  help resolve any issues.
>> What do others think?
>> Specific ideas on comment guidelines? Javadoc-style comments for  
>> APIs/ SPIs, etc? What types of comments should be expected?
>> --kevan
>>

Re: Writing Readable Code

[Joe Bohn <jo...@earthlink.net>]

I completely agree.

There are some areas of the code that contain virtually no comments. 
Any comments are better than none.  Better still would be some 
guidelines as you've suggested.

At a minimum I think that each method should have javadoc style comments 
that actually explain the purpose of the method as opposed to simply 
echoing the obvious (param name/type definitions and return types).  I 
think that descriptive comments highlighting the purpose of the method 
are most helpful.  I also prefer block comments at critical places in 
the code to describe the flow over EOL comments which are easily missed.

Joe


Kevan Miller wrote:
> During the recent discussions regarding the Geronimo Development  
> process, several people expressed some concern about moving away from  
> RTC.  The biggest issue seemed to be that RTC insured multiple people  
> reviewed new code. Having reviewed the code, the reviewers now  
> understood and would be able to support the code (i.e. fix bugs).
> 
> This is certainly a valid concern. However, even though we're now  
> following CTR, we all need to be making a concerted effort to provide  
> the same level of review as commits are made.
> 
> No matter what process we're following, IMO, the best way to insure  
> that people are reading *and* understanding your code is to write  code 
> that is easy to read and understand. This does not mean writing  simple 
> code. It simply means keeping the reader in mind and trying to  make 
> their job easier.
> 
> The single, most important thing, in my mind, is to provide clear and  
> insightful comments to assist the reader. These don't need to be  
> verbose tomes. They don't need to state the obvious. However, any  
> assistance you can provide the reader is helpful. Describe the  
> processing flow that methods are being invoked. What are the  threading 
> assumptions? Identify subtleties that a reader might not be  aware of. 
> Who are the potential callers? etc...
> 
> In case anyone is wondering -- I think we've been lacking in this  
> department. I'd like to see simple comment guidelines incorporated  into 
> the Documentation Guidelines for our CTR process.
> 
> In my opinion, failure to appropriately comment new code is cause for  a 
> commit to be vetoed. I doubt that this will happen often. I expect  that 
> in most instances these issues can be resolved appropriately  through 
> simple discussion. Some basic rules-of-thumb are likely to  help resolve 
> any issues.
> 
> What do others think?
> 
> Specific ideas on comment guidelines? Javadoc-style comments for APIs/ 
> SPIs, etc? What types of comments should be expected?
> 
> --kevan
>  
> 

Re: Writing Readable Code

[anita kulshreshtha <a_...@yahoo.com>]

     I have read a lot of code and found that the code is
understandable enough. However I found it was hard to get an overall
picture. 
   I agree with Kevan that "When a  method is a part of an overall
larger picture, pulling that context  together can be extremely useful
to the reader". Matt suggested that "for the person who is looking at
the code six months from now it might be nice to have part of that
discussion on the dev list condensed into the code in the form of a
comment". IMHO, it would be better to put this condensed discussion in
the Jira issue that was used to commit the code. The code should have a
link to this issue. We should make sure that this informal design
document includes all the explanations/issues raised by the community
but still be high level enough to be not affected by minor tweaks in
the implementation. A finished document for the wiki is even better but
not required.
   Other than this I agree with Paul that " low bar for committing to
trunk should be that it compiles, does no harm, and has undergone
adequate discussion within the Dev community.  Providing adequate
comments lies just outside that threshold as something that you should
expect to be nagged heavily about but not necessarily vetoed."

Thanks
Anita

--- Kevan Miller <ke...@gmail.com> wrote:

> During the recent discussions regarding the Geronimo Development  
> process, several people expressed some concern about moving away from
>  
> RTC.  The biggest issue seemed to be that RTC insured multiple people
>  
> reviewed new code. Having reviewed the code, the reviewers now  
> understood and would be able to support the code (i.e. fix bugs).
> 
> This is certainly a valid concern. However, even though we're now  
> following CTR, we all need to be making a concerted effort to provide
>  
> the same level of review as commits are made.
> 
> No matter what process we're following, IMO, the best way to insure  
> that people are reading *and* understanding your code is to write  
> code that is easy to read and understand. This does not mean writing 
> 
> simple code. It simply means keeping the reader in mind and trying to
>  
> make their job easier.
> 
> The single, most important thing, in my mind, is to provide clear and
>  
> insightful comments to assist the reader. These don't need to be  
> verbose tomes. They don't need to state the obvious. However, any  
> assistance you can provide the reader is helpful. Describe the  
> processing flow that methods are being invoked. What are the  
> threading assumptions? Identify subtleties that a reader might not be
>  
> aware of. Who are the potential callers? etc...
> 
> In case anyone is wondering -- I think we've been lacking in this  
> department. I'd like to see simple comment guidelines incorporated  
> into the Documentation Guidelines for our CTR process.
> 
> In my opinion, failure to appropriately comment new code is cause for
>  
> a commit to be vetoed. I doubt that this will happen often. I expect 
> 
> that in most instances these issues can be resolved appropriately  
> through simple discussion. Some basic rules-of-thumb are likely to  
> help resolve any issues.
> 
> What do others think?
> 
> Specific ideas on comment guidelines? Javadoc-style comments for
> APIs/ 
> SPIs, etc? What types of comments should be expected?
> 
> --kevan
>   
> 


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com