105. Construct Binary Tree from Preorder and Inorder Traversal #31
Conversation
| # self.val = val | ||
| # self.left = left | ||
| # self.right = right | ||
| class Solution: |
There was a problem hiding this comment.
これちょっと読みにくかったです。理由はっきりと分からないんですが、append_next_candidatesが何にappendするのかが全体から探さないと分からないからかもしれないと思いました。
関数に分けてるのも個人的には無い方が読みやすいかなと思いました。目の移動が多いデメリットが勝ったのかもしれないです
There was a problem hiding this comment.
コメントありがとうございます。
append_next_candidatesは名前を横着しましたかね。。
一方で、関数化は書く側としては頭が整理できてよかったのかなと感じています（というか一気に書き下すと、うまく書けなかった気がします）
| node.right = TreeNode(right_preorder[0]) | ||
| node_and_order_pairs.append((node.right, right_preorder, right_inorder)) | ||
|
|
||
| root = TreeNode(preorder[0]) |
There was a problem hiding this comment.
Step1では空配列のケアをしていますが、こちらと1つ下の解法ではしていないですね
| return build_tree_helper(0, len(preorder)) | ||
| ``` | ||
| 思考ログ： | ||
| - preorder_indexを内部で1ずつ増やしていくのがやや分かりにくいか |
There was a problem hiding this comment.
このタイミングでpreorderのindexズラしていって問題ないんだっけ、と少し詰まりました。
There was a problem hiding this comment.
自分も理解に時間がかかりました。preorderのindexのstartとendを渡してあげると良さそうです。
| ``` | ||
| 思考ログ： | ||
| - ```.index```で捜索したり、スライスしたりと効率的ではないのだけど、シンプルで分かりやすい気はする | ||
| - ```preorder```のスライスに使ってる```inorder_split_index```は不親切かも、冗長でも```num_left_subtree_node```とかに入れておいた方がいいかもしれない |
There was a problem hiding this comment.
````inorder_split_index```は不親切かも
自分もそう思いました。わざわざ感ありますが、inorderを左右に分割する関数定義してもよさそうです
There was a problem hiding this comment.
コメントありがとうございます。
短く書けてリーズナブルな気がしたので敢えて手を加えなかったのですが、もう少し丁寧にやっても良かったですね。
| return build_tree_helper(0, len(preorder)) | ||
| ``` | ||
| 思考ログ： | ||
| - preorder_indexを内部で1ずつ増やしていくのがやや分かりにくいか |
There was a problem hiding this comment.
自分も理解に時間がかかりました。preorderのindexのstartとendを渡してあげると良さそうです。
| inorder_value_to_index = {value: index for index, value in enumerate(inorder)} | ||
|
|
||
| preorder_index = 0 | ||
| def build_tree_helper(inorder_left_bound: int, inorder_right_bound: int) -> TreeNode: |
There was a problem hiding this comment.
細かいですが、return type は Optional[TreeNode] ですかね。
| return None | ||
|
|
||
| root_val = preorder[0] | ||
| inorder_split_index = inorder.index(root_val) |
There was a problem hiding this comment.
この時点で変数名をleft_subtree_sizeとして良いかと思いました。
そうすると例えば下で、preorder[1:1+left_subtree_size] となり、インデックス1から長さが左部分木のサイズと等しいだけ切り出していることが一目でわかります。
| val = preorder[0] | ||
| inorder_split_index = inorder.index(val) | ||
|
|
||
| left_tree_inorder = inorder[:inorder_split_index] |
There was a problem hiding this comment.
スライスをつくるより、インデックスの範囲を表す整数を用いたほうが、コピーが発生せず、処理が軽くなると思います。
| def buildTree(self, preorder: List[int], inorder: List[int]) -> Optional[TreeNode]: | ||
| inorder_value_to_index = {value: index for index, value in enumerate(inorder)} | ||
|
|
||
| preorder_index = 0 |
There was a problem hiding this comment.
個人的には、使った preorder_index の数を build_tree_helper が一緒に返す形にするかなあと思いました。
https://leetcode.com/problems/construct-binary-tree-from-preorder-and-inorder-traversal/description/