Skip to content

Implemented the Binary Search Tree Exercise#182

Merged
rmonnet merged 5 commits intoexercism:mainfrom
rmonnet:ex-binary-search-tree
Feb 5, 2026
Merged

Implemented the Binary Search Tree Exercise#182
rmonnet merged 5 commits intoexercism:mainfrom
rmonnet:ex-binary-search-tree

Conversation

@rmonnet
Copy link
Contributor

@rmonnet rmonnet commented Feb 3, 2026

For some reason, the problem spec shows tests verifying the
structure of the tree nodes and other tests verifying the
tree content is sorted.

Since we leave it to the student to implement the Node data
structure we can't really check the structure of the tree itself.
The set of tests checking the structure of the tree after various
combinations of `insert()` is redundant with the tests checking
the tree is properly sorted since the only way to get the values
sorted is to have the tree structure correct so we skiped these
redundant tests (and marked them so in tests.toml).

The alternative would be to provide the structure of the Node to
the student to ensure we can test it. This doesn't seem as
interesting an exercise.

For some reason, the problem spec shows tests verifying the
structure of the tree nodes and other tests verifying the
tree content is sorted.

Since we leave it to the student to implement the Node data
structure we can't really check the structure of the tree itself.
The set of tests checking the structure of the tree after various
combinations of `insert()` is redundant with the tests checking
the tree is properly sorted since the only way to get the values
sorted is to have the tree structure correct so we skiped these
redundant tests (and marked them so in tests.toml).

The alternative would be to provide the structure of the Node to
the student to ensure we can test it. This doesn't seem as
interesting an exercise.
@rmonnet rmonnet added x:action/create Work on something from scratch x:type/content Work on content (e.g. exercises, concepts) labels Feb 3, 2026
@rmonnet rmonnet requested a review from glennj February 3, 2026 20:11
@glennj
Copy link
Contributor

glennj commented Feb 4, 2026

I disagree. There's really only one way to implement a binary search tree. I fear if we don't enforce some structure, then students will just fall back to a dynamic array sorted.

Maybe instead of having to spit out a string representation of the whole tree, if the tests are written like the ruby track with chained "left" and "right" accessors, it might be a bit more interesting.

@rmonnet
Copy link
Contributor Author

rmonnet commented Feb 4, 2026

Yes, I thought about somebody just using a dynamic array and the sort function, but being an optimist, I thought "surely nobody would do that!". Anyway :-)

I looked at the Ruby track and I think it is easier for them because Ruby is an interpreted language and the test fail, not really because a compile error but with a "NoMethodError: undefined method `left' for an instance of Bst" message. That's kind of the same but it is more graceful and we have been trying not to show the student compile errors in the Odin exercises (not on a solution stub anyway).

I can definitively go ahead and add the definition of the Node. If I do that, I can definitively implement the tests I skipped and if they were to switch to a dynamic array then they would get compile errors (but not on a solution stub which was our goal).

If I don't see any objection, I'll work that out tomorrow.

Included the definition of Node in the solution stub, this is
acceptable since "there is only one way to implement a binary
search tree".

With the Node definition, we can re-include the five skipped
tests that just check that the constructed tree has the
expected structure for all constructed nodes.

Remove the include/comment from tests.toml that documented the
skipped tests.

Note that the resulting additional tests are way messier than in
a language like Ruby. The main reasons are:
1. Odin tests are more verbose than ruby tests
(`testing.expect...()`)
2. We need to deal with null nodes in the tree. Each node we want
to check needs to be non-nil or this will crash the test.
@rmonnet
Copy link
Contributor Author

rmonnet commented Feb 4, 2026

@glennj, see if you like this version better. I added the Node definition in the solution stub and implemented the 5 tests dealing with the tree structure.

@glennj
Copy link
Contributor

glennj commented Feb 4, 2026

I think the expected [something] to be nil assertions can be removed. As long as all the inserted values can be located, I think the students will be OK.

@rmonnet rmonnet added the x:size/large Large amount of work label Feb 5, 2026
@rmonnet
Copy link
Contributor Author

rmonnet commented Feb 5, 2026

@glennj, all done!

@rmonnet rmonnet merged commit 2776e76 into exercism:main Feb 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

x:action/create Work on something from scratch x:size/large Large amount of work x:type/content Work on content (e.g. exercises, concepts)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants