bubbletree icon indicating copy to clipboard operation
bubbletree copied to clipboard

Error on node with 2 children

Open tgillet1 opened this issue 12 years ago • 6 comments

Perhaps I misunderstood something in how to create an appropriate structure, but I found that when I had just 2 children for a node that an error occurred on line 523 of bubbletree.js. I found that on line 181 if node.right == node.left (when there are two nodes at the level), node.right is set to undefined. Then on line 523 node.right.amount is accessed but node.right is undefined, causing the error. I simply included a node.right==undefined check around the statement calculating rad2 and removed the node.right.amount component of the Math.max call in that case. My test case worked after making that change.

tgillet1 avatar Apr 08 '12 17:04 tgillet1

For those of you without programming skills, here's the solution in code. Thanks a lot, tgillet1!

            if (node.right==undefined) {
                rad2 = 0 - Math.max(
                    //hw *0.8 - tgtScale * (a2rad(node.parent.amount)+a2rad(node.amount)), // maximum visible part
                    hw * 0.8 - tgtScale * (a2rad(node.parent.amount) + a2rad(Math.max(node.amount*1.15 + node.maxChildAmount*1.15, node.left.amount * 0.85))),
                    tgtScale*a2rad(node.parent.amount)*-1 + hw*0.15 // minimum visible part
                ) + hw;
            }
            else {
            rad2 = 0 - Math.max(
                    //hw *0.8 - tgtScale * (a2rad(node.parent.amount)+a2rad(node.amount)), // maximum visible part
                    hw * 0.8 - tgtScale * (a2rad(node.parent.amount) + a2rad(Math.max(node.amount*1.15 + node.maxChildAmount*1.15, node.left.amount * 0.85, node.right.amount * 0.85))),
                    tgtScale*a2rad(node.parent.amount)*-1 + hw*0.15 // minimum visible part
                ) + hw;
            }

PhilippFS avatar Sep 01 '12 09:09 PhilippFS

works great so far. thanks

mrgcohen avatar Sep 07 '12 17:09 mrgcohen

Thank you very much!

fedossov avatar Sep 27 '12 17:09 fedossov

Thanks, this really helped!

colinchan avatar Mar 21 '13 21:03 colinchan

I just ran into this same problem, and also spent a while thinking I was doing it wrong! I've done a slightly different fix that handles both undefined left and right nodes, and opened a PR. Works for me (TM).

Floppy avatar Apr 23 '13 16:04 Floppy

Floppy's fix is a little more elegant: less lines of code, and works for more use cases. I'll add it here for reference.

    rad2 = 0 - Math.max(
      hw * 0.8 - tgtScale * (a2rad(node.parent.amount) + a2rad(Math.max(node.amount*1.15 + node.maxChildAmount*1.15, (node.left ? node.left.amount : 0) * 0.85, (node.right ? node.right.amount : 0)  * 0.85))),
      tgtScale*a2rad(node.parent.amount)*-1 + hw*0.15 // minimum visible part
    ) + hw;

crates avatar Jun 30 '14 19:06 crates