Thanks for getting that list of examples together. That's a pretty good mix!
I went through these too without looking at Todd's comments first to avoid
prejudice. Here's my results..
1) ugly dangling ')'
6-7) would prefer 4 spaces before 'throws'
11-12) ok.
16-17) ok. I don't think we should mandate one of 11-12 over 16-17 or vice
versa. Both seem good.
22-23) ok. In the same spirit as the former.
27-28) I think indent should be "four spaces" or "align to the (", but not
some other random distance.
32--35) A bit C-ish, but okay? Would prefer to condense this to look like
16--17
39--46) please don't staircase
50--52) Would prefer one fewer space on those; indent only to the '('
56--59) The dangling ')' again looks lonely.
62--67) This just combines two styles I've previously been unhappy about.
So I think we're mostly in accordance.
I'm less happy about the example on 32--35 than he is, but I also don't
think it's a bad thing necessarily.
I also second the vote for wikification of examples (both good and bad).
While we're on the subject of style, and, without trying to open a *huge*
can of worms... I've got two other grievances worth mentioning:
First, I've been picked on by others for using this brace style:
if (foo) {
stmt;
} else {
otherstmt;
}
and have been told to drop the braces because they look "ugly" if stmt or
otherstmt are only one line.
In http://java.sun.com/docs/codeconv/html/CodeConventions.doc6.html#449though,
the sun coding conventions *clearly* say that braces are always to
be used. Can we get a ruling here? My vote is to do as the SCC says.
(Rationale: if we later expand otherstmt to two lines, or expand stmt to
include a nested if, we might accidentally inject a bug.) If we're going to
allow "dangling one-liners" though, we need to clearly state this deviation
in our own style guide.
And second, what's our story on tabs vs. spaces? In
http://java.sun.com/docs/codeconv/html/CodeConventions.doc3.html#262 they
claim "The exact construction of the indentation (spaces vs. tabs) is
unspecified." So much for standards! :P I know that we have a
well-established guideline that we indent by 2s, not by 4s. But beyond that,
I've always used spaces all around. My .vimrc includes:
set tabstop=2
set shiftwidth=2
set expandtab
Various Hadoop sources mix tabs and spaces -- sometimes on the same line!
Moving forward, can we pick one or the other for all new/modified source?
The Hadoop style guidelines do not specify one or the other. My vote is for
spaces all around. At least within the MapReduce codebase, it seems as
though there's only a few instances of tabs really remaining:
aa...@jargon:~/src/git/mapred/src/java$ ack '\t' | wc -l
144
.. so it seems as though the consensus is pretty existent. I wouldn't mind
if we formally codified it though.
- Aaron
On Fri, Nov 20, 2009 at 11:01 AM, Konstantin Boudnik <c...@yahoo-inc.com>wrote:
> > So, IMO, the goal should be the examples on 10-24 or 31-36.
>
> +1 I agree with Todd: the highlighted snippets are most appropriate as Java
> coding style.
>
>
> On 11/20/09 10:54 , Todd Lipcon wrote:
>
>> My opinions on the groups of line numbers from that pastebin:
>>
>> 1-3: Definitely not - no reason for ) on its own line
>> 5-8: no, "throws" should be indented
>> 10-13: I think this is acceptable
>> 15-19: also acceptable IMO
>> 22-24: acceptable - lines wrapped due to column limit should indent their
>> wrappings
>> 26-29: bad
>> 31-36: seems the same as 16-19, so OK
>> 39-47: bad - bizarre
>> 50-53: bad - seems like a mistaken space
>> 56-59: same as 1-3, seems bizarre
>> 62-66: also bizarre like 50-53
>>
>> So, IMO, the goal should be the examples on 10-24 or 31-36.
>>
>> If others agree, perhaps we should put some of these examples on the wiki?
>>
>> -Todd
>>
>> On Fri, Nov 20, 2009 at 9:04 AM, Cosmin Lehene<cleh...@adobe.com> wrote:
>>
>> Hi,
>>>
>>> I was trying to make a patch and looking over the Hadoop guidelines for
>>> code at http://wiki.apache.org/hadoop/CodeReviewChecklist, trying to
>>> follow the conventions.
>>>
>>> Looking through code I found a few "patterns", however, these differ even
>>> in the same class sometimes. Here's a collection of method declaration
>>> styles I gathered from 2 or 3 files: http://pastie.org/707389
>>>
>>> I have to admit Sun's code conventions are a bit unclear too, so maybe a
>>> list of settings for the IDE like tab size, indent, continuation indent,
>>> exceptions to the rule, etc. would help.
>>>
>>> Cosmin
>>>
>>>
