Skip to content
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

binary-search-tree: test_inserting_same may be inaccurate/confusing #861

Closed
BradleyJohnson opened this issue Sep 22, 2018 · 3 comments
Closed

Comments

@BradleyJohnson
Copy link

  def test_inserting_same
    four = Bst.new 4
    four.insert 4
    assert_equal 4, four.data
    assert_equal 4, four.left.data
  end

As I understand it, this test asserts for the behavior of a binary tree rather than a binary search tree. Adding a duplicate value should probably throw an exception or simply drop the duplicate, right? Happy to open a PR for this if folks agrees.

@Insti
Copy link
Contributor

Insti commented Sep 22, 2018

What does the canonical data say?

But it looks like these tests are not generated from the canonical data.

Any PR that improves things is strongly encouraged.

There's another issue about this exercise, maybe you also have an opinion on that:
#513

@Insti Insti changed the title BST test_inserting_same may be inaccurate/confusing binary-search-tree: test_inserting_same may be inaccurate/confusing Sep 22, 2018
@BradleyJohnson
Copy link
Author

@Insti Canonical data agrees with the current implementation. I suppose the behavior of duplicate inserts is dependant on application rather than some strict interpretation of the data structure. I'll close this, sorry for the noise!

@Insti
Copy link
Contributor

Insti commented Sep 23, 2018

It is always possible that the canonical-data is incorrect.

Thanks for making an issue anyway ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants