You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "David Bowen (JIRA)" <ji...@apache.org> on 2007/01/29 20:46:49 UTC

[jira] Created: (HADOOP-948) Coding style issues

Coding style issues 
--------------------

                 Key: HADOOP-948
                 URL: https://issues.apache.org/jira/browse/HADOOP-948
             Project: Hadoop
          Issue Type: Bug
          Components: metrics
            Reporter: David Bowen
            Priority: Minor


I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:

   * It is generally preferable to avoid multiple return statements.
   * It is nearly always preferable to use curly braces and a newline after an if (condition).
   * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).

(1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 

I'll attach the fix shortly.


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Re: [jira] Commented: (HADOOP-948) Coding style issues

Posted by Doug Cutting <cu...@apache.org>.
Nigel Daley wrote:
>> Currently, when I run checkstyle (with sun's style conventions, which 
>> we use for Hadoop) I get tons of warnings on almost each and every 
>> source file.
> 
> ...which is perhaps one reason it is too late to use CheckStyle.

We can't use CheckStyle, but not for that reason: it's LGPL.

Also, we use Sun's style conventions with at least one modification: we 
indent two spaces per level, not four.

Style changes should be done as separate commits and with some 
hesitation, since they can break patch files and make it difficult to 
trace the history of a file.

Doug

Re: [jira] Commented: (HADOOP-948) Coding style issues

Posted by Nigel Daley <nd...@yahoo-inc.com>.
> On a related note, we should really use CheckStyle (http:// 
> checkstyle.sourceforge.net) to enforce the coding styles.  
> Currently, when I run checkstyle (with sun's style conventions,  
> which we use for Hadoop) I get tons of warnings on almost each and  
> every source file.

...which is perhaps one reason it is too late to use CheckStyle.

>
>
>> Coding style issues
>> --------------------
>>
>>                 Key: HADOOP-948
>>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>>             Project: Hadoop
>>          Issue Type: Bug
>>          Components: metrics
>>            Reporter: David Bowen
>>            Priority: Minor
>>
>> I would like to recommend some mainly stylistic changes in the  
>> recent fix of http://issues.apache.org/jira/browse/HADOOP-886.   
>> The file in question is CodeFactory.java, and the reasons for the  
>> changes are:
>>    * It is generally preferable to avoid multiple return statements.
>>    * It is nearly always preferable to use curly braces and a  
>> newline after an if (condition).
>>    * There's no benefit to doing the hash lookup twice in the  
>> common case (by calling contains and then get).
>> (1) and (2) are commonly found in Java coding style guidelines as  
>> they make the code more readable.
>> I'll attach the fix shortly.
>
> -- 
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>


[jira] Commented: (HADOOP-948) Coding style issues

Posted by "Milind Bhandarkar (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12468385 ] 

Milind Bhandarkar commented on HADOOP-948:
------------------------------------------

On a related note, we should really use CheckStyle (http://checkstyle.sourceforge.net) to enforce the coding styles. Currently, when I run checkstyle (with sun's style conventions, which we use for Hadoop) I get tons of warnings on almost each and every source file.


> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HADOOP-948) Coding style issues

Posted by "David Bowen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

David Bowen updated HADOOP-948:
-------------------------------

    Status: Patch Available  (was: Open)

The patch is available.

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HADOOP-948) Coding style issues

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12468402 ] 

Doug Cutting commented on HADOOP-948:
-------------------------------------

CheckStyle is LGPL, so cannot be included in an Apache project.

I would not oppose someone submitting a patch that adds style checking to the "test" target, so that tests fail if style is incorrect.  In general, the "test" target should test for patch acceptability.

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HADOOP-948) Coding style issues

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12470724 ] 

Hadoop QA commented on HADOOP-948:
----------------------------------

+1, because http://issues.apache.org/jira/secure/attachment/12349851/hadoop-948.patch applied and successfully tested against trunk revision r503864.

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HADOOP-948) Coding style issues

Posted by "David Bowen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12471789 ] 

David Bowen commented on HADOOP-948:
------------------------------------

The rest of the metrics package also uses 4-space indentation.  It would not be useful to change just one method in one class to use the Hadoop 2-space indentation convention - the whole package should be changed at once.  At the same time, the excess carriage-return characters that got in there should be removed, but there shouldn't be any code changes.

Alternatively, we could consider switching hadoop over to the de-facto industry standard which is a 4-space indentation.  


> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HADOOP-948) Coding style issues

Posted by "David Bowen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

David Bowen updated HADOOP-948:
-------------------------------

    Attachment:     (was: hadoop-948.patch)

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HADOOP-948) Coding style issues

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doug Cutting updated HADOOP-948:
--------------------------------

    Status: Open  (was: Patch Available)

This purports to improve the style, yet it does not conform to Hadoop's stated style guidelines.  In particular, Hadoop prefers code that indents two spaces per level, and this patch adds new code that indents four-spaces per level.

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Re: [jira] Commented: (HADOOP-948) Coding style issues

Posted by Doug Cutting <cu...@apache.org>.
David Bowen wrote:
> To return to the original point though, we should have some minimum set
> of conventions, and that needs to be enforced.

We do have a convention: Sun's except with 2-space indentation.  It 
isn't currently strictly enforced.  Perhaps it ought to be, but too much 
rigidity in this regard can cause more strife than it alleviates.

> People like me who have
> additional conventions on top of the minimum will just have to put up
> with seeing their code changed in ways that they don't like.

Right, collaboration requires accepting some diversity, not trying to 
force everyone to be like one's self.  Civil societies may vary in their 
ranges of permitted expression, but some tolerance of difference is 
required.

Doug

Re: [jira] Commented: (HADOOP-948) Coding style issues

Posted by David Bowen <db...@yahoo-inc.com>.
> There is clearly a discrepancy in sun's coding style conventions,
> because exceptions, which are multiple-level returns are allowed to be
> thrown any number of times in a single unit.
I think you mis-read my comment: I said that Sun's conventions do NOT
support my preferred style.  They don't say anything about multiple returns.

To return to the original point though, we should have some minimum set
of conventions, and that needs to be enforced.  People like me who have
additional conventions on top of the minimum will just have to put up
with seeing their code changed in ways that they don't like.



Re: [jira] Commented: (HADOOP-948) Coding style issues

Posted by Milind Bhandarkar <mi...@yahoo-inc.com>.
I agree with Owen that multiple returns (e.g. checking the validity  
of arguments first, and returning error codes, before doing the  
operation) do not hamper the readability of the code. There is  
clearly a discrepancy in sun's coding style conventions, because  
exceptions, which are multiple-level returns are allowed to be thrown  
any number of times in a single unit.

- Milind

On Feb 6, 2007, at 9:31 AM, David Bowen wrote:

>
>> Irrational fear of a second return has lead to lots of unnecessary  
>> code branches, in my experience. I'd rather see a return early in  
>> a function that a nested if branch that includes the rest of the  
>> body of the function.
>>
> I think it is entirely rational to avoid second returns.  (In cases
> where there are a lot of early-exit tests, I would agree that multiple
> returns are better than multiple nested if-then-elses.)  However, I
> looked up the Sun Java coding conventions and found no mention of  
> this,
> so I don't know of any authority to which I can appeal to support my
> case.  To me, code is more readily understandable if each unit has a
> single exit point.
>
>
>

--
Milind Bhandarkar
(mailto:milindb@yahoo-inc.com)
(phone: 408-349-2136 W)



Re: [jira] Commented: (HADOOP-948) Coding style issues

Posted by David Bowen <db...@yahoo-inc.com>.
> Irrational fear of a second return has lead to lots of unnecessary code branches, in my experience. I'd rather see a return early in a function that a nested if branch that includes the rest of the body of the function.
>   
I think it is entirely rational to avoid second returns.  (In cases
where there are a lot of early-exit tests, I would agree that multiple
returns are better than multiple nested if-then-elses.)  However, I
looked up the Sun Java coding conventions and found no mention of this,
so I don't know of any authority to which I can appeal to support my
case.  To me, code is more readily understandable if each unit has a
single exit point.



[jira] Commented: (HADOOP-948) Coding style issues

Posted by "Owen O'Malley (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12470459 ] 

Owen O'Malley commented on HADOOP-948:
--------------------------------------

Irrational fear of a second return has lead to lots of unnecessary code branches, in my experience. I'd rather see a return early in a function that a nested if branch that includes the rest of the body of the function.

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (HADOOP-948) Coding style issues

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doug Cutting resolved HADOOP-948.
---------------------------------

       Resolution: Duplicate
    Fix Version/s: 0.12.0

This was included in HADOOP-954.

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>             Fix For: 0.12.0
>
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HADOOP-948) Coding style issues

Posted by "David Bowen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

David Bowen updated HADOOP-948:
-------------------------------

    Attachment: hadoop-948.patch

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HADOOP-948) Coding style issues

Posted by "David Bowen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12471794 ] 

David Bowen commented on HADOOP-948:
------------------------------------

Oops, I think we sneaked this change in accidentally in another patch.  Sorry.  This issue can be closed now.


> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HADOOP-948) Coding style issues

Posted by "Tom White (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12470368 ] 

Tom White commented on HADOOP-948:
----------------------------------

Could the "test" target use CheckStyle? I notice Apache Jetspeed uses it (http://portals.apache.org/jetspeed-1/code-standards.html).

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HADOOP-948) Coding style issues

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12470380 ] 

Doug Cutting commented on HADOOP-948:
-------------------------------------

> Could the "test" target use CheckStyle?

Sure.  We cannot include the checkstyle jar in svn, so it should be optional.

> I notice Apache Jetspeed uses it

Looking at:

http://svn.apache.org/viewvc/portals/jetspeed-1/trunk/build/build.xml?view=markup

Checkstyle looks to be optional and is only used if the developer has independently downloaded and installed checkstyle.  One could instead make build.xml download checkstyle, but that's frowned on.

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HADOOP-948) Coding style issues

Posted by "David Bowen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

David Bowen updated HADOOP-948:
-------------------------------

    Attachment: hadoop-948.patch

> Coding style issues 
> --------------------
>
>                 Key: HADOOP-948
>                 URL: https://issues.apache.org/jira/browse/HADOOP-948
>             Project: Hadoop
>          Issue Type: Bug
>          Components: metrics
>            Reporter: David Bowen
>            Priority: Minor
>         Attachments: hadoop-948.patch
>
>
> I would like to recommend some mainly stylistic changes in the recent fix of http://issues.apache.org/jira/browse/HADOOP-886.  The file in question is CodeFactory.java, and the reasons for the changes are:
>    * It is generally preferable to avoid multiple return statements.
>    * It is nearly always preferable to use curly braces and a newline after an if (condition).
>    * There's no benefit to doing the hash lookup twice in the common case (by calling contains and then get).
> (1) and (2) are commonly found in Java coding style guidelines as they make the code more readable. 
> I'll attach the fix shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.