Skip to content

105. Construct Binary Tree from Preorder and Inorder Traversal#29

Open
seal-azarashi wants to merge 12 commits intomainfrom
construct-binary-tree-from-preorder-and-inorder-traversal
Open

105. Construct Binary Tree from Preorder and Inorder Traversal#29
seal-azarashi wants to merge 12 commits intomainfrom
construct-binary-tree-from-preorder-and-inorder-traversal

Conversation

@seal-azarashi
Copy link
Owner

- イテレーションごとに inorder の要素を線形探索せずに済むように map を使っている
- 可読性向上のためにメンバー変数を使って書いている
- public メソッドの buildTree() は何度呼ばれても同じ結果を返すために初期化処理を入れた
- 【🚨ご意見ください】ここでメンバー変数を使うことについて、ネガティブな意見があれば教えてもらえますと嬉しいです
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner Author

@seal-azarashi seal-azarashi Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oda
返信遅くなりました。
ああ、確かに仰る通りですね。どうも Leetcode 上の実行環境に思考が囚われて、それ以外の環境で実行した際にどんな懸念があるか、といった考えが出づらくなっているなと反省しました... 別の指摘で外部ライブラリを使うのはどうかと指摘を頂いたりもしましたが、そういった発想が出てくるようにしないとですね。

class Solution {
public TreeNode buildTree(int[] preorder, int[] inorder) {
// Make it an array so that it could behave as reference type value
int[] preorderIndex = new int[]{ 0 };
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java だと Box 化された Integer を使うのが普通かしら。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

返信遅くなりました。
Integer は参照型ではあるのですが、実は immutable なのでこの行の int[] 型の代替にはならないんですよね...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

わお。Java あんまり触っていないのがばれますねえ。
https://stackoverflow.com/questions/4520137/does-java-have-mutable-types-for-integer-float-double-long
を見ると、候補は
int[]
java.util.concurrent.atomic.AtomicInteger
org.apache.commons.lang3.mutable.MutableInt
あたりみたいですね。

- イテレーションごとに inorder の要素を線形探索せずに済むように map を使っている
- 可読性向上のためにメンバー変数を使って書いている
- public メソッドの buildTree() は何度呼ばれても同じ結果を返すために初期化処理を入れた
- 【🚨ご意見ください】ここでメンバー変数を使うことについて、ネガティブな意見があれば教えてもらえますと嬉しいです
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- メンバー変数はクラスから生成されるオブジェクトが保持する状態のようなものだと認識している
- しかしここではあくまで buildTree() の実装を見やすくする用途で用いられているのが懸念
- 賛否両論ありそうなので意見を聞いてみたい
- あまり素直じゃない実装になってしまった印象で、配列の部分コピーを使用するパターンの方が読みやすい気がする
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

たしかに、このコードは素直ではないですね。
preorder と preorderIndex という近いものの引き回し方が引数とメンバ変数という異なる方法でなされているのが大きそうです。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かにそうですね。この実装の直下にある、メンバー変数を使わないで書いた方は、メンバ変数を使わず全て引数として引き回しているので幾分か読みやすく感じます。

return buildTree(preorder, 0, inorder.length, preorderIndex, valToInorderIndex);
}

private TreeNode buildTree(int[] preorder, int left, int right, int[] preorderIndex, Map<Integer, Integer> valToInorderIndex) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preorderIndex を共有して触るのはあまり好ましくないと考えるならば、
もう一つの考え方として、buildTree が TreeNode とともに、返り値でサイズを報告するようにすることです。
ここからいくつ使ったかが分かるので、次にどこから見たらいいかが分かります。

preorderIndex を共有する方法は、パーサーなどでは稀に見る形ではあります。
preorder と preorderIndex でクラスにしてしまう(つまり、preorder 順に一要素ずつ出てくる箱のようなものだと思う)ということもできます。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oda
すいませんこちらも確認遅くなりました。

preorder と preorderIndex でクラスにしてしまう(つまり、preorder 順に一要素ずつ出てくる箱のようなものだと思う)ということもできます。

なるほどです。こちら試しに実装してみましたが、preorder と preorderIndex (クラス内では cursor としました) の関係性が明確になってより良い実装になったと思います。

class Solution {
    private static class PreorderWithCursor {
        private final int[] preorder;
        private int cursor;

        PreorderWithCursor(int[] preorder) {
            this.preorder = preorder;
            this.cursor = 0;
        }
    }

    public TreeNode buildTree(int[] preorder, int[] inorder) {
        Map<Integer, Integer> valToInorderIndex = new HashMap<>();
        for (int i = 0; i < inorder.length; i++) {
            valToInorderIndex.put(inorder[i], i);
        }
        PreorderWithCursor preorderWithCursor = new PreorderWithCursor(preorder);
        return buildTreeHelper(preorderWithCursor, 0, inorder.length, valToInorderIndex);
    }
    
    private TreeNode buildTreeHelper(PreorderWithCursor preorderWithCursor, int left, int right, Map<Integer, Integer> valToInorderIndex) {
        if (left == right) {
            return null;
        }

        int val = preorderWithCursor.preorder[preorderWithCursor.cursor++];
        int inorderIndex = valToInorderIndex.get(val);
        TreeNode node = new TreeNode(val);
        node.left = buildTreeHelper(preorderWithCursor, left, inorderIndex, valToInorderIndex);
        node.right = buildTreeHelper(preorderWithCursor, inorderIndex + 1, right, valToInorderIndex);
        return node;
    }
}

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

もう一つの考え方として、buildTree が TreeNode とともに、返り値でサイズを報告するようにすることです。
ここからいくつ使ったかが分かるので、次にどこから見たらいいかが分かります。

こちらも実装してみました。一つ前の実装よりもトリッキーで理解しづらい印象です。

class Solution {
    private record TreeNodeWithSize(TreeNode node, int size) {}

    public TreeNode buildTree(int[] preorder, int[] inorder) {
        Map<Integer, Integer> valToInorderIndex = new HashMap<>();
        for (int i = 0; i < inorder.length; i++) {
            valToInorderIndex.put(inorder[i], i);
        }
        return buildTreeWithSize(preorder, 0, inorder.length, 0, valToInorderIndex).node;
    }
    
    private TreeNodeWithSize buildTreeWithSize(int[] preorder, int left, int right, int preorderStart, Map<Integer, Integer> valToInorderIndex) {
        if (left == right) {
            return new TreeNodeWithSize(null, 0);
        }
        
        int val = preorder[preorderStart];
        TreeNode node = new TreeNode(val);
        int inorderIndex = valToInorderIndex.get(val);

        TreeNodeWithSize leftSubtree = buildTreeWithSize(preorder, left, inorderIndex, preorderStart + 1, valToInorderIndex);
        TreeNodeWithSize rightSubtree = buildTreeWithSize(preorder, inorderIndex + 1, right, preorderStart + 1 + leftSubtree.size, valToInorderIndex);

        node.left = leftSubtree.node;
        node.right = rightSubtree.node;

        return new TreeNodeWithSize(node, 1 + leftSubtree.size + rightSubtree.size);
    }
}

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preorderIndex を共有する方法は、パーサーなどでは稀に見る形ではあります。

なるほど勉強になります。こういう知識の引き出しがあるのすごいなと思いつつ、自分ももっといろんな実装を読んでいかないとなと思いました...

上記理解した上で、次のようなことを考えながら実装していました:

- Complete binary tree でないかもしれないのでスタックオーバーフローが起こる懸念があるが、現時点の自分の理解度だとループを用いた実装を綺麗に書ける自信がないので、問題文の constraints が守られることを前提に再帰処理で書いてみよう
- 引数について、 null でないことやそれぞれ要素数が等しいといった前提で処理を書いているが、全部コメントに残すと見た目が煩雑になりそうなのでひとまず書かないでおこう
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java のアノテーションで @NotNull というのがありますね。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かに Lombok 等のアノテーションライブラリが使えればかなり綺麗にかけますね。 Leetcode は使えないので考えとして出てきませんでしたが、そういった選択肢もあるよねっていうのはちゃんと面接の場では言えるようにしたいなと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(今は Lombok よりも org.jetbrains.annotations の方が人気なのかな)

// 時間計算量: O(n^2):
// - O(n): ノード生成処理を引数に渡される配列の要素数と同じ回数実行
// - O(n^2): 各イテレーションで、新規配列を作成するため、引数に渡された配列の合計要素数 (配列の要素数 - 1) * 2 と同じ回数のコピー処理を行う
// 空間計算量: O(n): 配列が複数作成され、それら要素数の合計は引数に渡される配列たちの合計要素数より少ない
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

最悪の場合、O(n) のスタックフレームが O(n) 個積み上がると O(n^2) になりませんか?

int inorderMiddle = 0;
for (int i = 0; i < preorder.length; i++) {
if (preorder[0] == inorder[i]) {
inorderMiddle = i;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

break してよさそうです。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かに。修正しました: 0770261


TreeNode node = new TreeNode(preorder[0]);
int inorderMiddle = 0;
for (int i = 0; i < preorder.length; i++) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

細かいですが、この書き方だと preorder の方を走査しているように見えるのが気になりました。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同じく修正しました: 0770261

seal_azarashi added 2 commits October 16, 2024 08:53
2. 配列 inorder から、 1 と同じ要素が格納される index を見つける
3. 2 のインデックスを用いて preorder, inorder を分割し、左右の子ノードを生成するためそれぞれ次のイテレーションに渡す
- 2のインデックスは左子ツリーの要素数と対応する
- preorder は 先頭要素がルートノードなので、それを含めない [1:2のインデックス+1] の範囲を左、[2のインデックス+1:preorder.length] の範囲を右の子ツリーとする
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[] は閉区間を表しているように見えます。もしそうであれば、左の範囲は [1:2のインデックス] となると思います。同様に右の範囲は [2のインデックス+1:preorder.length-1] となると思います。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます。恥ずかしながら数学に明るくないので先程調べたのですが、[] は閉区間、() は開区間と ISO に定められているのですね。
Go だと上記の書き方で半開区間 (左閉右開) の指定が出来るので、その感覚で書いていました (参考)。

Copy link
Owner Author

@seal-azarashi seal-azarashi Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nodchip
ちなみに、以下のように記載すれば正確に意図が伝わりますでしょうか?

「preorder は 先頭要素がルートノードなので、それを含めない [1,2のインデックス+1) の区間を左、[2のインデックス+1,preorder.length) の区間を右の子ツリーとする」

インタビュー対策というよりは数学の質問になってしまい恐縮ですが...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

はい、正確に意図が伝わっているように思います。

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

数学も表記のゆれはあって ]a,b[ という表記をしたり、a..b を使ったりすることもあります。
Python も [:] で左を含み右を含まないですし、[:] で意図はそれなりに汲んでもらえるものと私は思います。
通じなかったら、上でしたように注釈をつけるなり相談して変えるなりしましょう。

https://en.wikipedia.org/wiki/Ellipsis_(computer_programming)#Ranges
.. を使う言語も、Bash Perl Ruby Haskell Rust など結構ありますね。

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R, Matlab, Julia は 1:10 で10を含むようです。
Haskell の [1..10] は10を含みます。
Bash の {1..10} は10を含みます。
Perl も (1..10) で10を含みます。
Ruby は、(1..10) は10を含み、(1...10)は10を含みません。
Swift は、(1...10) で含み、(1..<10) で含みません。もともとは、(1..10) で含んでいました。
Rust は、1..=10 が含み、1..10 で含みません。

色々ですね。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nodchip @oda
お二人共ありがとうございます!

oda さん、参考情報もありがとうございます。
こうやって並べられて見るとかなり言語ごとに違いますね。数学でも表記揺れがあるのも仰る通りですし、通じるか通じないかについては、相手によって必要であれば都度確認が必要に思いました。

あとこれは感想ですが、自分でも上記 oda さんが調べられているぐらいは自ら調査をしておいた方が良かったなと思いました。考え方や振る舞い、いつも参考にさせてもらっています。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そういえば今 cszap で読んでいる『システム設計の面接試験』では半開区間 (左閉右開) が [1,2のインデックス+1) のように書かれてましたね

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ひとまず今回は [1,2のインデックス+1) のような表記にしてみました。
d35386b

3. 2 のインデックスを用いて preorder, inorder を分割し、左右の子ノードを生成するためそれぞれ次のイテレーションに渡す
- 2のインデックスは左子ツリーの要素数と対応する
- preorder は 先頭要素がルートノードなので、それを含めない [1:2のインデックス+1] の範囲を左、[2のインデックス+1:preorder.length] の範囲を右の子ツリーとする
- inorder は [0:2のインデックス] の範囲が左、[2のインデックス+1:inorder.length] の範囲が右の子ツリーとなる
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちらも、左の範囲は [0:2のインデックス-1]、右の範囲は [2のインデックス+1:inorder.length-1] となると思います。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

メモ: こちらの返答内容に従って修正する: #29 (comment)


- Complete binary tree でないかもしれないのでスタックオーバーフローが起こる懸念があるが、現時点の自分の理解度だとループを用いた実装を綺麗に書ける自信がないので、問題文の constraints が守られることを前提に再帰処理で書いてみよう
- 引数について、 null でないことやそれぞれ要素数が等しいといった前提で処理を書いているが、全部コメントに残すと見た目が煩雑になりそうなのでひとまず書かないでおこう
- 配列の分割には Arrays.copyOfRange() を使うのが良さそうだ
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

配列のコピーを作るより、元の配列を引き回し、インデックスで管理するほうが、処理コストが低くなることが多いと思います。そちらを優先することをおすすめいたします。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

仰るとおりですね。初見でここの計算量の考慮が出来てなかったのは反省ポイントです。以後気をつけます。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants