[
https://issues.apache.org/jira/browse/HADOOP-16671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16961958#comment-16961958
]
Lisheng Sun commented on HADOOP-16671:
--------------------------------------
Thank [~hexiaoqiao] for your comments.
it is necessary to keep strict args checks consider InnerNodeImpl#getLeaf is
one public method.
And since InnerNodeImpl#getLeaf is one public method,
{code:java}
if (excludedIndex != -1 && leafIndex >= 0) {
// excluded node is one of the children so adjust the leaf index
leafIndex = leafIndex>=excludedIndex ? leafIndex+1 : leafIndex;
}
{code}
excludedIndex could be equal to children.size(), when leafIndex=
excludedIndex, leafIndex is out of index.
This situation can be handled specially, leafIndex= excludedIndex- 1
Please correct me if i was wrong. Thank you.
> Optimize InnerNodeImpl#getLeaf
> ------------------------------
>
> Key: HADOOP-16671
> URL: https://issues.apache.org/jira/browse/HADOOP-16671
> Project: Hadoop Common
> Issue Type: Improvement
> Reporter: Lisheng Sun
> Assignee: Lisheng Sun
> Priority: Major
> Attachments: HADOOP-16671.001.patch
>
>
> {code:java}
> @Override
> public Node getLeaf(int leafIndex, Node excludedNode) {
> int count=0;
> // check if the excluded node a leaf
> boolean isLeaf = !(excludedNode instanceof InnerNode);
> // calculate the total number of excluded leaf nodes
> int numOfExcludedLeaves =
> isLeaf ? 1 : ((InnerNode)excludedNode).getNumOfLeaves();
> if (isLeafParent()) { // children are leaves
> if (isLeaf) { // excluded node is a leaf node
> if (excludedNode != null &&
> childrenMap.containsKey(excludedNode.getName())) {
> int excludedIndex = children.indexOf(excludedNode);
> if (excludedIndex != -1 && leafIndex >= 0) {
> // excluded node is one of the children so adjust the leaf index
> leafIndex = leafIndex>=excludedIndex ? leafIndex+1 : leafIndex;
> }
> }
> }
> // range check
> if (leafIndex<0 || leafIndex>=this.getNumOfChildren()) {
> return null;
> }
> return children.get(leafIndex);
> } else {
> {code}
> the code InnerNodeImpl#getLeaf() as above
> i think it has two problems:
> 1.if childrenMap.containsKey(excludedNode.getName()) return true,
> children.indexOf(excludedNode) must return > -1, so if (excludedIndex != -1)
> is it necessary?
> 2. if excludedindex = children.size() -1
> as current code:
> leafIndex = leafIndex>=excludedIndex ? leafIndex+1 : leafIndex;
> leafIndex will be out of index and return null. Actually there are nodes that
> can be returned.
> i think it should add the judgement excludedIndex == children.size() -1
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
