3 ms·

> Don't constructors always have side effects? The only side effect they should have is creating a new instance, no playing around with some other object's hie

> Don't constructors always have side effects? 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.

dahart 7y ago
> 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.
- ehsankia 7y ago
  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?
  - dahart 7y ago
    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. :)

dahart 7y ago

> 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.

ehsankia 7y ago
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?
- dahart 7y ago
  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. :)