New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integer names breaks functionality silently #6

Closed
cromulus opened this Issue May 28, 2012 · 1 comment

Comments

Projects
None yet
2 participants
@cromulus

cromulus commented May 28, 2012

When you create nodes with specifying an integer as the name of the node, finding a node like so @tree[node_name] results in the node at the index, not the node named as the integer. The test below fails:

   @root = Tree::TreeNode.new("ROOT", "Root Node")

      @child1 = Tree::TreeNode.new("Child1", "Child Node 1")
      @child2 = Tree::TreeNode.new("Child2", "Child Node 2")
      @child3 = Tree::TreeNode.new("Child3", "Child Node 3")
      @child4 = Tree::TreeNode.new("Child4", "Grand Child 1")
      @child5 = Tree::TreeNode.new("Child5", "Child Node 4")

      @n_root = Tree::TreeNode.new(0, "Root Node")

      @n_child1 = Tree::TreeNode.new(1, "Child Node 1")
      @n_child2 = Tree::TreeNode.new(2, "Child Node 2")
      @n_child3 = Tree::TreeNode.new(3, "Child Node 3")
      @n_child4 = Tree::TreeNode.new(4, "Grand Child 1")
      @n_child5 = Tree::TreeNode.new(5, "Child Node 4")


      @root << @child1
      @root << @child2
      @root << @child3 << @child4

      @n_root << @n_child1
      @n_root << @n_child2
      @n_root << @n_child3 << @n_child4

      assert_equal(@root['Child1'].name,'Child1') #passes
     assert_equal(@n_root[1].name,1) #fails

Perhaps integers should be explicitly disallowed as node names?

@evolve75

This comment has been minimized.

Show comment
Hide comment
@evolve75

evolve75 Aug 19, 2012

Owner

Thanks for reporting the issue! The problem is that the TreeNode.[] is an overloaded method, which actually uses the integer argument to retrieve the nth child. In the scenario above, @n_root[1].name is actually returning the 2nd child or the root (since the access is zero based, as for arrays). The test will pass if you use:

 assert_equal(@n_root[0].name, 1)

However, I do agree that this is confusing. I think adding a warning message during the node creation will be better. Also, an optional flag in the TreeNode[] method can enable the use case you are looking for.

Expect to see the documentation change (and the optional flag based feature) in R0.8.3.

Owner

evolve75 commented Aug 19, 2012

Thanks for reporting the issue! The problem is that the TreeNode.[] is an overloaded method, which actually uses the integer argument to retrieve the nth child. In the scenario above, @n_root[1].name is actually returning the 2nd child or the root (since the access is zero based, as for arrays). The test will pass if you use:

 assert_equal(@n_root[0].name, 1)

However, I do agree that this is confusing. I think adding a warning message during the node creation will be better. Also, an optional flag in the TreeNode[] method can enable the use case you are looking for.

Expect to see the documentation change (and the optional flag based feature) in R0.8.3.

@ghost ghost assigned evolve75 Aug 19, 2012

@evolve75 evolve75 closed this Aug 19, 2012

evolve75 added a commit that referenced this issue Aug 20, 2012

evolve75 added a commit that referenced this issue Aug 20, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment