Conversation
| - 返り値を格納する変数名は levelOrderTraversal にしない方が良さそう (書いてる自分がよくわからなくなるので) | ||
| - node が empty になると constraints にあるのでこれに対処しなければ | ||
| - リストの実装型は LinkedList が良さそう | ||
| - 後ろに要素を追加していくだけなので、インデックスを使うためのオーバーヘッドが多い ArrayList は除外 |
There was a problem hiding this comment.
LinkedListよりArrayListの方が多くの場合において速いと思います。
There was a problem hiding this comment.
ご指摘ありがとうございます。
これについてですが、 LinkedList では要素追加の度にメモリ領域が確保されるのが主な原因と考えたのですが、こちらいかが思われますでしょうか？ ArrayList の末尾への要素追加はインデックスの追加も伴いますが、メモリ領域の確保が行われる回数は少ないので、結果として早くなるのかなと推測しています。あと確保されるメモリ領域が連続しているのも要素追加処理の効率に寄与していそうです。
There was a problem hiding this comment.
ArrayListの実装はDynamic arrayになっています。リンクの記事を確認してみてください。
| while (!nodes.isEmpty()) { | ||
| int currentLevelNodeCount = nodes.size(); | ||
| List<Integer> valsInCurrentLevel = new LinkedList<>(); | ||
| for (int i = 0; i < currentLevelNodeCount; i++) { |
There was a problem hiding this comment.
nodes は、同じレベルの値を返り値に詰めるために使い、また次のレベルのノードを格納するという2つの役割を持っていると思います。
次のレベルのノードを格納する役割を別の Queue<TreeNode> に持たせると、役割の分担という意味でより分かりやすくなる気がします。
There was a problem hiding this comment.
これ私も賛成ですね。nodes の中に何が入っているかを自然言語で表現すると、割と面倒なことになるはずです。
「currentLevelNodeCount - i 個は、一つ前のレベルが入っていて、そこから残りは次のレベルが入っている」っていうことになりますね。
There was a problem hiding this comment.
@goto-untrapped @oda
すいません、対応が大変遅くなりました。
仰るとおり nodes が複数の役割を持った複雑なものになっていたので、次のレベルのノードを格納する方の役割を新しく宣言した newNodes に任せるようにしました。実装は step 4 として別途記載しています: 8ed4aa8
自然言語での表現も素直になったと思います:
nodesには現イテレーションで走査対象になるレベルが入っているnewNodesには次のイテレーションで走査対象になるレベルが入っている
| } | ||
|
|
||
| valsInEachLevel.get(level).add(node.val); | ||
| int nextLevel = level + 1; |
There was a problem hiding this comment.
あえてnextLevelに入れなくても意図は伝わるのかなと思いました。全体的に綺麗なコートだと思いました。
There was a problem hiding this comment.
すいません、対応が大変遅くなりました。
ありがとうございます！確かに意図は伝わりますね。ただこちら2箇所で使うので、コードを書いてる際に修正が必要になった際に修正が容易になるように残しておきたいなと思いました。
こういうケースで修正が必要になった際、一部だけ修正を忘れて長い時間詰まるみたいな経験を何度かしてるので、そういったことが起こりづらいようにしたいという意図もあります。
|
|
||
| ### 再帰関数を用いた DFS アプローチ | ||
|
|
||
| スタックオーバーフローのリスクはありますが、練習で書いてみました。 |
| } | ||
|
|
||
| private void findValuesInEachLevel(TreeNode node, int level, List<List<Integer>> valsInEachLevel) { | ||
| if (valsInEachLevel.size() == level) { |
There was a problem hiding this comment.
ちょっとifだと意図が伝わりづらい気もします(参照)
また、==なのに加えるとなると一瞬「なんでだっけ？」となる気もするので、右辺をlevel + 1にして<とかにしてもいい気もしますが、難しいですね
There was a problem hiding this comment.
返信遅くなり失礼しました。リンク先の内容含めて非常に納得出来る指摘でしたので、採用して step 4 に記載しました: 92352eb
また、==なのに加えるとなると一瞬「なんでだっけ？」となる気もするので、右辺をlevel + 1にして<とかにしてもいい気もしますが、難しいですね
これは悩ましいですね... ひとまず今の while 文を使った実装が、「level が大きくて valsInEachLevel が足りない場合、足りるように拡張する」と説明出来る素直な実装になるので、これが考えられる中でベストなのかなと思います。
| return valsInEachLevel; | ||
| } | ||
|
|
||
| private void findValuesInEachLevel(TreeNode node, int level, List<List<Integer>> valsInEachLevel) { |
7 participants
https://leetcode.com/problems/binary-tree-level-order-traversal/description/