Add 103. Binary Tree Zigzag Level Order Traversal.md #27
Conversation
| ) -> None: | ||
| if node is None: | ||
| return | ||
| nonlocal zigzag_traversed_values |
There was a problem hiding this comment.
When the definition of a function or class is nested (enclosed) within the definitions of other functions, its nonlocal scopes are the local scopes of the enclosing functions.
とあるので、なくても問題ありません。上から下に読んだ時に、急に初期化のされていない変数がでないようという意図です。
There was a problem hiding this comment.
この問題で、nonlocalを書かないとエラーになる理由が、やっと何となくわかりました。
https://github.com/olsen-blue/Arai60/pull/29/files#:~:text=return%20None-,nonlocal%20preorder_index,-preorder_index%20%2B%3D%201
nonlocalなしでpreorder_index = preorder_index + 1と代入(bound)の形で書くと、preorder_indexがローカル変数扱いになるということだったんですね。そんなローカル変数はないぞ、というエラー(UnboundLocalError)が実際出ていました。
nonlocalを書くと解決できていました。
https://docs.python.org/3/reference/executionmodel.html#naming-and-binding:~:text=If%20a%20name%20is%20bound%20in%20a%20block%2C%20it%20is%20a%20local%20variable%20of%20that%20block%2C%20unless%20declared%20as%20nonlocal%20or%20global.
今回のzigzag_traversed_valuesは、変数自体を更新しているわけではなく、変数自体に要素追加の更新をしているだけなので、nonlocal不要ということですかね。
ざっくりいうと、イミュータブルなものを更新したければnonlocalが必要で、ミュータブルなものの更新(要素追加など)であれば、nonlocal不要ということでしょうか。
There was a problem hiding this comment.
ちらっと書かれていますが、mutable, immutableというより、より厳密にはinner functionのなかでbind(今回はassignment)があるかどうかな気がします(各種bind)。
関数block内で定義されたオブジェクトのスコープは、（inner functionの定義内を含め）その関数定義内全てとなる。ただ、(inner functionなど)内部に新たなblockができ、そこで同じ名前で新たなbindを導入すると、その名前に紐づいたobjectは内部のblock内に限られる。(原文, blockの定義)
従って整理すると以下のような感じかと思います
- inner functionの中では、読み込み専用で使う分には(bindがないので)外側の関数の値を見ることができる。
- 書き込みをしたい場合、immutableだと代入以外に変数名に紐づいた値を更新できないが、代入をするとその変数のスコープはinner function内に限られる。
- nonlocalをつけることで、外側のスコープのものを引っ張ってこれるので書き込みも可能になる
- mutableだと代入以外の方法でも中身の書き換えができるので、結果としてnonlocalを使わずに済む場合がある。
実際に言語化するといい生理になりますね。コメントありがとうございます
There was a problem hiding this comment.
補足ありがとうございます。bindとは何かがわかっていなかったです。読んでみます。
| zigzag_traversed_values = [] | ||
| nodes_in_level = deque([root]) | ||
| start_from_left = True | ||
| while True: |
There was a problem hiding this comment.
個人的にループ条件を指定できるのであればしたいなという気持ちになりました。仮にもっと長いソースで何か変更して無限ループになった時、ソースを読む負担が減るんじゃないかなと思いました。
| def zigzagLevelOrder(self, root: Optional[TreeNode]) -> List[List[int]]: | ||
| level_ordered_values = [] | ||
| nodes_in_level = [root] | ||
| while True: |
There was a problem hiding this comment.
trueだとループの終了条件を読み取る時にbreakのあるところまで読む必要があり、コードを読む量が増えて読みづらくなってしまいます。
nodes_in_levelがemptyではない、という条件で良さそうに思います
There was a problem hiding this comment.
nodes_in_level に None のみが含まれている場合にもループを抜けたいのだと思います。そのため、直後の行で None を取り除き、そのあとで空かどうか判定しているのだと思います。
There was a problem hiding this comment.
補足ありがとうございます。
おっしゃる通り、nodes_in_levelには、Noneも気にせず入れていいという感じでやっています。したがって、この書き方で、L206のTrueをnot nodes_in_levelとしてもその条件が成立することはないので、そこだけを変更してもかえって複雑かなと思ってます。
ただ、ループ始まってすぐのL207実行後にnodes_in_levelにNoneが含まれなくなる(変数の意味が少し変わる)こと、L207自体が少し複雑な書き方をしていること、指摘されている通りbreakかreturnを探さないといけないことなど、そもそもこの構造自体がすこし複雑だったかもなと今は思っています。
（書いた時は、なかなかうまく書けたんじゃないかと思ったんですが見返すと厳しいですね。いつもコメントありがとうございます）
There was a problem hiding this comment.
nodes_in_level に None のみが含まれている場合にもループを抜けたいのだと思います
おっしゃる通り、nodes_in_levelには、Noneも気にせず入れていいという感じでやっています。
コメントありがとうございます、ちゃんと全部読めていなかったです。
| nodes_in_next_level.append(node.left) | ||
| nodes_in_next_level.append(node.right) | ||
| nodes_in_level = nodes_in_next_level | ||
| level_ordered_values.append(values_in_level) |
There was a problem hiding this comment.
ここで if not start_from_left: values_in_level.reverse() とすると下がいらなくなりますね。
| if level % 2: | ||
| zigzag_traversed_values[level].append(node.val) | ||
| else: | ||
| zigzag_traversed_values[level].appendleft(node.val) |
There was a problem hiding this comment.
深さごとの部屋 deque() に入れる時の方向をappend, appendleft で切り替えるんですね。なるほどです。
| class Solution: | ||
| def zigzagLevelOrder(self, root: Optional[TreeNode]) -> List[List[int]]: | ||
| zigzag_traversed_values = [] | ||
| nodes_in_level = deque([root]) |
There was a problem hiding this comment.
先頭からの pop() をしていないため、 deque である必要がないように感じました。より軽い list を使ったほうがよいと思います。
| nodes_in_level = deque([root]) | ||
| start_from_left = True | ||
| while True: | ||
| nodes_in_level = deque(filter(None, nodes_in_level)) |
There was a problem hiding this comment.
None を取り除いているということが分かりにくく感じました。
nodes_in_level に入れる前に None であるかをチェックしてから入れたほうが分かりやすいと思います。あるいは、 nodes_in_level に入れ終わった直後にこの行を入れ、かつコメントで処理内容を補足するとよいと思いました。
| def zigzagLevelOrder(self, root: Optional[TreeNode]) -> List[List[int]]: | ||
| level_ordered_values = [] | ||
| nodes_in_level = [root] | ||
| while True: |
There was a problem hiding this comment.
nodes_in_level に None のみが含まれている場合にもループを抜けたいのだと思います。そのため、直後の行で None を取り除き、そのあとで空かどうか判定しているのだと思います。
| node = nodes_in_level.pop() | ||
| values_in_level.append(node.val) | ||
| nodes_in_next_level.appendleft(node.right) | ||
| nodes_in_next_level.appendleft(node.left) |
There was a problem hiding this comment.
個人的には、本当に逆順で入っているかの確認のためにL29から32とL36から39をじっくり見比べる必要があり、あまり好みではないです。(depthが奇数の時reverseする、などの処理だけの方が読む負担が少ない)
| nodes_in_level = list(filter(None, nodes_in_level)) | ||
| if not nodes_in_level: | ||
| break | ||
| nodes_in_level.reverse() |
There was a problem hiding this comment.
これも結構読む時の認知負荷がありますね。
ここで毎回reverse()した結果、nodes_in_levelの処理とnode_in_next_levelへのappend処理がどの順で行われているのか追うのが大変でした
| continue | ||
| while not len(zigzag_traversed_values) > level: | ||
| zigzag_traversed_values.append(deque()) | ||
| if level % 2: |
There was a problem hiding this comment.
この書き方少し不安になります。
言語による差もあるので比較演算子を使うほうが無難かなと思います。
https://zenn.dev/bugbearr/articles/69e1101ccc676a4b28b6
https://leetcode.com/problems/binary-tree-zigzag-level-order-traversal/description/