Recently a co-worker stumpled upon the following bug which was caused by two TreeViewNode instances with identical values. It was fairly difficult to figure out the cause of the problem so it would be beneficial to have TreeView and other controls throw an error when duplicate values are detected:
Reproduction URL: https://jsfiddle.net/6b97w83L/3/
IMPORTANT: Test with "Pause on caught exceptions" enabled in Developer Tools!
Test:
Select an item in the dropdown. The selected value is emitted to the browser console and the control is disposed, as it is no longer needed. Unfortunately it throwns an error, and it is not at all obvious why it fails. The reason is explained in the screenshot below.
"Recently" we introduced the childrenIndex collection in TreeViewNode which lets us lookup children more effectively through GetChild("222", true) ("222" = node value, true = recursively) on a TreeView or TreeViewNode instance. But childrenIndex must be in sync with the nodes registered in childrenArray, and since the first can't contain multiple nodes with the same value, and the latter can, internal state becomes inconsistent and things break as we see.
We should consider detecting duplicate values. However, nodes might not all be connected to the combined node tree initially, and entire partial node trees can even be combined at different points in time, each containing values also declared in another partial node tree. So what we need is something that always iterates the entire (combined) node tree, every time a new node is added. Something like (NOT TESTET):
// POC: Protect against duplicate node values
var topNode = me;
while (topNode.GetParent())
{
topNode = topNode.GetParent();
}
if (topNode.GetChild(node.Value(), true) !== null)
{
throw "Duplicate node value detected";
}
Whenever a node is added anywhere in a collection, the entire collection will be checked for duplicate keys. However, make absolute sure this is close to free in terms of CPU time !! Test with a TreeView with 100 top nodes, each with 10 child nodes, each with 5 child nodes, each with 2 child nodes (= 10.000 nodes in total).
Alternatively, we should make sure that at least a node's local instance of childrenIndex and associated childrenArray are protected against duplicates which would probably resolve the bug reported here. However, TreeView might still produce unexpected results for nodes with duplicate values across different levels - for instance it would not be entirely clear which node with a duplicate value is selected, removed, manipulated, etc.
Marking issue as as bug despite control being used incorrectly. It should not be this difficult to figure out why something breaks, and it should certainly not break this easily.
Check ListView which might also allow for duplicate values - fortunately this is just a flat structure.
Recently a co-worker stumpled upon the following bug which was caused by two TreeViewNode instances with identical values. It was fairly difficult to figure out the cause of the problem so it would be beneficial to have TreeView and other controls throw an error when duplicate values are detected:
Reproduction URL: https://jsfiddle.net/6b97w83L/3/ IMPORTANT: Test with "Pause on caught exceptions" enabled in Developer Tools!
Test: Select an item in the dropdown. The selected value is emitted to the browser console and the control is disposed, as it is no longer needed. Unfortunately it throwns an error, and it is not at all obvious why it fails. The reason is explained in the screenshot below.
"Recently" we introduced the childrenIndex collection in TreeViewNode which lets us lookup children more effectively through
GetChild("222", true)("222" = node value, true = recursively) on a TreeView or TreeViewNode instance. But childrenIndex must be in sync with the nodes registered in childrenArray, and since the first can't contain multiple nodes with the same value, and the latter can, internal state becomes inconsistent and things break as we see.We should consider detecting duplicate values. However, nodes might not all be connected to the combined node tree initially, and entire partial node trees can even be combined at different points in time, each containing values also declared in another partial node tree. So what we need is something that always iterates the entire (combined) node tree, every time a new node is added. Something like (NOT TESTET):
Whenever a node is added anywhere in a collection, the entire collection will be checked for duplicate keys. However, make absolute sure this is close to free in terms of CPU time !! Test with a TreeView with 100 top nodes, each with 10 child nodes, each with 5 child nodes, each with 2 child nodes (= 10.000 nodes in total).
Alternatively, we should make sure that at least a node's local instance of childrenIndex and associated childrenArray are protected against duplicates which would probably resolve the bug reported here. However, TreeView might still produce unexpected results for nodes with duplicate values across different levels - for instance it would not be entirely clear which node with a duplicate value is selected, removed, manipulated, etc.
Marking issue as as bug despite control being used incorrectly. It should not be this difficult to figure out why something breaks, and it should certainly not break this easily.
Check ListView which might also allow for duplicate values - fortunately this is just a flat structure.