[ 
https://issues.apache.org/jira/browse/HADOOP-16156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16798121#comment-16798121
 ] 

Daniel Templeton commented on HADOOP-16156:
-------------------------------------------

After a careful review, I have a couple more comments:
# Looks like this change was unnecessary:{code}  return new 
InnerNodeImpl(parentName,
        getPath(this), this, this.getLevel()+1);{code}  That line was not over 
the 80 char limit.  Now, if I were in a mood to reformat code, I'd add a space 
on either side of that plus, which would put it over the limit.  You can go 
either way, but the change in the current patch is superfluous.
# If you're going to reformat this line:{code}    if (loc == null || 
loc.length() == 0) return this;{code} then you may as well put some parens 
around the clauses in the conditional.
# If you're going to reformat these lines:{code}    if (childnode == null) 
return null; // non-existing node
    if (path.length == 1) return childnode;{code} you should probably combine 
them into a single conditional.  If {{childnode == null}}, then {{return 
childnode}} is the same as {{return null}}.  If you do, be sure to add a 
comment to explain it.

> [Clean-up] Remove NULL check before instanceof and fix checkstyle in 
> InnerNodeImpl
> ----------------------------------------------------------------------------------
>
>                 Key: HADOOP-16156
>                 URL: https://issues.apache.org/jira/browse/HADOOP-16156
>             Project: Hadoop Common
>          Issue Type: Improvement
>            Reporter: Shweta
>            Assignee: Shweta
>            Priority: Minor
>         Attachments: HADOOP-16156.001.patch, HADOOP-16156.002.patch, 
> HADOOP-16156.003.patch
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to