Add level order traversal Kotlin goody #369
Conversation
willmadison
force-pushed
the
kt-level-order-traversal
branch
from
5879847 to
078d7b0
Compare
workspaces/adventure-pack/goodies/kotlin/src/traverse_level_order/Test.kt
Show resolved
Hide resolved
workspaces/adventure-pack/goodies/kotlin/src/traverse_level_order/Main.kt
Outdated
Show resolved
Hide resolved
| yield(current) | ||
|
|
||
| if (current.left != null) { | ||
| q.add(current.left!!) |
There was a problem hiding this comment.
I am not familiar with Kotlin either, and I was curious whether the non-null assertion is necessary, given that within the if condition this is bound to be non-null. The same for line 22.
| import common.TreeNode | ||
|
|
||
| public fun TreeNode?.traverseLevelOrder(): Sequence<TreeNode> { | ||
| if (this == null) return sequenceOf() |
There was a problem hiding this comment.
If I recall correctly, I came across a comment in a previous PR suggesting that for this repo we prefer wrapping return statements in braces. Let's check with Miorel if that's the style we should all adopt. In TypeScript, this can be added to the linter as a rule and auto-fixed, not sure what kind of lint is available for Kotlin.
There was a problem hiding this comment.
Thanks for the PR @willmadison! Kotlin is a breath of fresh air compared to Java sometimes 😄
I left a few comments, in particular I think we'd want a slightly different API for level order, that makes it easier to work with a level at a time. Let me know what you think.
workspaces/adventure-pack/goodies/kotlin/src/common/TreeNode.kt
Outdated
Show resolved
Hide resolved
workspaces/adventure-pack/goodies/kotlin/src/traverse_level_order/Test.kt
Outdated
Show resolved
Hide resolved
workspaces/adventure-pack/goodies/kotlin/src/traverse_level_order/Main.kt
Outdated
Show resolved
Hide resolved
|
Regarding the checks: you can take a look at the job summary (see for example https://github.com/code-chronicles-code/leetcode-curriculum/actions/runs/10514921554?pr=369) to see what failed. It looks like it's the snapshot tests. You will have to run Separately I see that you already discovered I prefer the more concise 2 space indentation rather than the 4, hope that's not too annoying for you 😄 |
workspaces/adventure-pack/goodies/kotlin/src/common/TreeNode.kt
Outdated
Show resolved
Hide resolved
…eNode>> rather than flattening it out on the caller's behalf.
There was a problem hiding this comment.
Thanks so much for the PR @willmadison! I had a few more comments but we can address those in a follow-up if you are interested.
|
|
||
| public data class TreeNode(val `val`: Int, var left: TreeNode? = null, var right: TreeNode? = null) | ||
|
|
||
| public fun TreeNode?.traverseLevelOrder(): Sequence<Collection<TreeNode>> { |
There was a problem hiding this comment.
Out of curiosity, any reason to prefer Collection over List for the return type?
| fun itTraversesLevelOrderSingleNode() { | ||
| val root: TreeNode = TreeNode(0) | ||
| assertContentEquals( | ||
| root.traverseLevelOrder().flatMap { it }.map { it.`val` }.toList(), listOf<Int>(0)) |
There was a problem hiding this comment.
I would imagine <Int> for the list type is inferred from the use of 0 as the value?
| TreeNode(2, TreeNode(4, TreeNode(8)), TreeNode(5)), | ||
| TreeNode(3, TreeNode(6), TreeNode(7))) | ||
| assertContentEquals( | ||
| root.traverseLevelOrder().flatMap { it }.map { it.`val` }.toList(), |
There was a problem hiding this comment.
For the most effective test, I think it would be helpful to do an assert on the levels here.
miorel
changed the title
Closes #314.