TheOdinProject / curriculum

The open curriculum for learning web development
https://www.theodinproject.com/
Other
9.06k stars 12.55k forks source link

Binary Search Trees project: levelOrder requiremement is not clear #27880

Open nguyenlekhtn opened 2 months ago

nguyenlekhtn commented 2 months ago

Checks

Describe your suggestion

image

The requirement asks to return an array of values if no callback is provided. But what should the function return if a callback is provided? Should it return an array of values too? Or it should return nothing if callback is provided?

My suggestion is that the function should always return an array of values no matter callback is provided or not.

Path

Ruby / Rails, Node / JS

Lesson Url

https://www.theodinproject.com/lessons/javascript-binary-search-trees

(Optional) Discord Name

haru => {haru:}

MaoShizhong commented 2 months ago

Thanks for opening this issue, @nguyenlekhtn. I've noticed this step is a common source of confusion amongst learners. The instruction's intention is to return an array of values only if no callback is provided. If a callback is provided, it should just be called on each Node as you traverse the tree, without returning anything. Basically like Array.prototype.forEach but with a default behaviour in the case of no callback.

However, this does feel a bit strange as the default behaviour is very different in nature to when a callback is provided. Since this (and the next step with the different traversal orders) is effectively like Array.prototype.forEach (only with a specific traversal order), I'm wondering if the instructions for this and the following step should be adjusted so that instead of returning an array of values in the case of no callback, it throws an error saying it expects a callback?

I believe this would make it more intuitive and also be more in line with how you'd expect such a method to work anyway. @TheOdinProject/ds-a thoughts?

MaoShizhong commented 1 month ago

Just adding on to the above, my suggestion regarding throwing an error if no callback is passed in (for levelOrder, inOrder, preOrder and postOrder i.e. steps 6 and 7) definitely makes sense for the JS version of this project, since it's much closer to Array.prototype.forEach.

Given that Ruby works a bit differently with things like returning Enumerators if no block's provided instead of throwing an error, I'm wondering then how best to approach the Ruby version. @TheOdinProject/ruby thoughts on this? Not familiar enough if something like always returning an array, block or not, would be the most ideal.

I know that for the JS version, it would make most sense to never return something from it and just throw if no callback provided.

github-actions[bot] commented 3 weeks ago

This issue is stale because it has had no activity for the last 30 days.