[
https://issues.apache.org/jira/browse/ZOOKEEPER-2680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15842908#comment-15842908
]
Edward Ribeiro commented on ZOOKEEPER-2680:
-------------------------------------------
A few comments:
{code}
private static Set<String> EMPTY_SET = Collections.unmodifiableSet(new
HashSet<String>(0));
{code}
is lacking the *final* modifier. Also, it can be:
{code}
private static final Set<String> EMPTY_SET = Collections.emptySet();
{code}
About {{DataNode.setChildren()}} (would love the comments of [~breed] and
[~fpj], as I can be about to write something utterly stupid :o) ):
{code}
public synchronized void setChildren(HashSet<String> children) {
this.children = children;
}
{code}
The code above could potentially pass a null. Is this behaviour expected? Say,
instead of providing a {{DataNode.clear()}} method we just pass
{{DataNode.setChildren(null)}} to reset all the children? Also, passing an
alien reference (children) that can be changed outside the scope of
{{DataNode}} seems potentially dangerous too. IMHO, {{DataNode.setChildren}}
could have been coded more defensively. I would have done something akin the
code below, but *of course* performance considerations should be taken into
account.
{code}
public synchronized void setChildren(Set<String> children) {
if (children == null || children.isEmpty()) // isEmpty() is optional
return;
if (this.children == null)
this.children = new HashSet<>(8);
this.children.addAll(children);
}
// new method
public synchronized void clear() {
children.clear();
}
{code}
All in all, I am +1 with this patch. :) Only took the opportunity to clarify
this {{DataNode.setChildren}} method use, but I think we can commit this patch
without changing the method just cited.
> Correct DataNode.getChildren() inconsistent behavior.
> -----------------------------------------------------
>
> Key: ZOOKEEPER-2680
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2680
> Project: ZooKeeper
> Issue Type: Bug
> Components: server
> Affects Versions: 3.4.9, 3.5.1
> Reporter: Mohammad Arshad
> Assignee: Mohammad Arshad
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2680-01.patch
>
>
> DataNode.getChildren() API returns null and empty set if there are no
> children in it depending on when the API is called. DataNode.getChildren()
> API behavior should be changed and it should always return empty set if the
> node does not have any child
> *DataNode.getChildren() API Current Behavior:*
> # returns null initially
> When DataNode is created and no children are added yet,
> DataNode.getChildren() returns null
> # returns empty set after all the children are deleted:
> created a Node
> add a child
> delete the child
> DataNode.getChildren() returns empty set.
> After fix DataNode.getChildren() should return empty set in all the above
> cases.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
