Don't constructors always have side effects? That's the only reason to have them.
I wouldn't think of this as a side-effect though, think of it as a graph property. addTo is setting up a tree structure, the same way you might setup a linked list. Think of the addTo property as a pointer to the parent, rather than a side-effect.
A doubly-linked list has two pointers per node, one for next node and one for previous. A tree node in a scene graph also has two pointers per node, one for parent and one for child. A parent can have multiple children, and so might arrange the children in an array. A child, however, can have only one parent. This makes setting the parent simpler than setting a child, considering corner cases like duplicate children.
Your suggestion is pointing at the addChild() form rather than addParent(). One of the other comments said that kind of call is available in Zdog, so maybe you can just use that instead.
But, I wouldn't use addRect() I would prefer addChild(). A call like addRect() is binding the type of shape and scene graph setup unnecessarily, so you'd have to provide separate calls for each shape type.
The only side effect they should have is creating a new instance, no playing around with some other object's hierarchy.
> think of it as a graph property. addTo is setting up a tree structure
The problem is that the hierarchy here is backwards. You usually add children to the parent, not parent to the children. Having a link back to your parent does help with navigating, but it shouldn't be how you "build" the tree.
When building a tree, do you think of it as building from children up to the parent? I did see that addChild was a thing, and that makes much more sense to me.
> The only side effect they should have is creating a new instance, no playing around with some other object’s hierarchy.
You might be making assumptions. The ‘addTo’ property does not imply the constructor is touching any other object.
You might be getting confused because the name is a verb. Again, addTo isn’t very standard naming, but what it represents is the parent property. You should think of it as a property that can define the graph, not assume that the name implies anything about the implementation. Even if the constructor does do something external to the node, the implementation can change.
> The problem is that the hierarchy here is backwards. You usually add children to the parent... it shouldn’t be how you “build” the tree.
No, it’s bottom-up, not backwards. And bottom-up builders are common in graphics, so I disagree with your assumption about how trees should be built. I think of building trees from child to parent all the time, because, as I mentioned before, it’s a simpler operation: all nodes have one and only one parent, where nodes can have any number of children and complications managing those children.
But the example calls render on `illo`, which is the parent. So at some point the "double link" between illo and the child was created, which is a side-effect, no?
Presumably updateGraph() and even render() can both tidy up linkages and anything else that needs updating. The shape constructor might well have a side-effect though, I don't know.
Hey I hear your point, this API is a little bit different. Maybe since it's early your feedback is fully justified. Just from my perspective it didn't strike me as "very strange", only perhaps a little bit. I don't think you're wrong, it's just a matter of degree... which is subjective anyway, so take this as lukewarm agreement with you. :)
I wouldn't think of this as a side-effect though, think of it as a graph property. addTo is setting up a tree structure, the same way you might setup a linked list. Think of the addTo property as a pointer to the parent, rather than a side-effect.
A doubly-linked list has two pointers per node, one for next node and one for previous. A tree node in a scene graph also has two pointers per node, one for parent and one for child. A parent can have multiple children, and so might arrange the children in an array. A child, however, can have only one parent. This makes setting the parent simpler than setting a child, considering corner cases like duplicate children.
Your suggestion is pointing at the addChild() form rather than addParent(). One of the other comments said that kind of call is available in Zdog, so maybe you can just use that instead.
But, I wouldn't use addRect() I would prefer addChild(). A call like addRect() is binding the type of shape and scene graph setup unnecessarily, so you'd have to provide separate calls for each shape type.