Skip to content

297. Serialize and Deserialize Binary Tree#74

Open
hayashi-ay wants to merge 5 commits intomainfrom
hayashi-ay-patch-63
Open

297. Serialize and Deserialize Binary Tree#74
hayashi-ay wants to merge 5 commits intomainfrom
hayashi-ay-patch-63

Conversation

@hayashi-ay
Copy link
Copy Markdown
Owner

```python
class Codec:

def serialize(self, root):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

自分なら DFS 版を serialize()/deserialize() 両方とも再帰で書くと思います。 serialize も再帰で書いてみていただけますか?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。書き直します。先にSerializeをループで書いて、Deserializeもループで書こうとしたら書けずに再帰で書いたという感じでした。



```python
class Codec:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

BFS 版は DFS 版に比べて、やや煩雑に感じました。おそらく DFS 版が想定解なのではないかと思います。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。LeetCodeの公式の解法もDFSでした。書いてみた感想として、探索だけならDFSもBFSもそこまで書きやすさは変わらないですが、Treeの構築が発生する場合はBFSおよびDFSのループだと書きにくいなと思いました。親ノードや左右どちらにつなぐかなどを考えないといけないので。

while nodes:
next_nodes = []
for node in nodes:
if node is None:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if not node:
の方が全体の統一感があると思いました。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

while nodesで判定しているから、ということですよね? nodesについてはlistでnodeはObjectなので、判定方法を揃える必要はないかなと思います。

listが空であることとObjectがNoneであることで違うことをしているが、書き方としては同じようにも書けるという話で無理に合わせる必要はないかなと思います。好みの問題だと思います。もちろんコーディング規約などがあればそちらに合わせはします。

nodes = deque([root])
while True:
parent = nodes.popleft()
try:
Copy link
Copy Markdown

@shining-ai shining-ai May 18, 2024

Choose a reason for hiding this comment

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

エラーを出すのが正常系ということに違和感を感じました。
ただ、良い直しかたを思いついてないです。すみません。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

この例外、大域脱出ではなく、next が動くかだけのためなので、単純に読みにくいと思います。next に default=None 渡すのでどうですか?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Pythonだと例外をキャッチするしかないと思ってたのですが、nextにdefaultを渡すことでStopIteration例外を投げる代わりにdefault値が変えるのですね。ありがとうございます。

https://docs.python.org/3/library/functions.html#next

if parent.right:
nodes.append(parent.right)
except StopIteration:
break
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

returnでも良いと思いました。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ある程度好みの問題かなと思います。while Trueの外側にもループがあればreturnにするとは思います。

@@ -0,0 +1,208 @@
Binary TreeのSerializeとDeserializeを行う問題。
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

serializeとdeserializeがほぼ同じ見た目になると分かりやすいのではないかと感じました。
BFS:

class Codec {
public:

    // Encodes a tree to a single string.
    string serialize(TreeNode* root) {
        ostringstream oss;
        queue<TreeNode*> nodes({root});
        while (!nodes.empty()) {
            auto node = nodes.front();
            nodes.pop();
            if (!node) {
                oss << "# ";
                continue;
            }
            oss << node->val << " ";
            nodes.push(node->left);
            nodes.push(node->right);
        }
        return oss.str();
    }

    // Decodes your encoded data to tree.
    TreeNode* deserialize(string data) {
        istringstream iss(data);
        TreeNode* res;
        queue<TreeNode**> nodes({&res});
        while (!nodes.empty()) {
            auto node = nodes.front();
            nodes.pop();
            string token;
            iss >> token;
            if (token == "#") {
                *node = nullptr;
                continue;
            }
            *node = new TreeNode(stoi(token));
            nodes.push(&(*node)->left);
            nodes.push(&(*node)->right);
        }
        return res;
    }
};

DFS:

class Codec {
public:

    // Encodes a tree to a single string.
    string serialize(TreeNode* root) {
        ostringstream oss;
        function<void(TreeNode*)> preorder = [&](auto node) {
            if (!node) {
                oss << "# ";
                return;
            }
            oss << node->val << " ";
            preorder(node->left);
            preorder(node->right);
        };
        preorder(root);
        return oss.str();
    }

    // Decodes your encoded data to tree.
    TreeNode* deserialize(string data) {
        TreeNode* res;
        istringstream iss(data);
        function<void(TreeNode*&)> preorder = [&](auto& node) {
            string token;
            iss >> token;
            if (token == "#") {
                node = nullptr;
                return;
            }
            node = new TreeNode(stoi(token));
            preorder(node->left);
            preorder(node->right);
        };
        preorder(res);
        return res;
    }
};

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。仰る通りだと思います。たとえば、仕様が変わった際に両方の処理が似たようなコードで書かれていると変更が容易になりそうだと思いました。

SerializeとDeserializeでどうしても違ってくるところは仕方がないと思いますが、共通して書けるところは同じような書き方をするという意識がそもそもなかったです。

特に2ndのDFSは先にSerializeをループで書いて、Deserializeもループで書こうとしたら書けずに再帰で書いたというのがあり、書き終わったタイミングで満足するのではなくSerializeを再帰で書き直せば良かったです。

values = []
depth = 0
nodes = [root]
while nodes:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

next_nodesがあるので、nodesは違いを明確にするため何か別の名前が良いか思いました。
same_level_nodesとかでしょうか。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

nodesは主役級の変数なのであえてsame_level_nodesなどと命名を長くしなくても良いと思うんですよね。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

これは趣味の範囲ですが、current_node は名前として長い感じがします。
ここのループにおいては主役なんだから、主役に、他と区別するためではない形容詞がついているのは不自然なんです。
「あなたもよっぽど分らないのね。だから天璋院様の御祐筆の妹の御嫁に行った先きの御っかさんの甥の娘なんだって、先っきっから言ってるんじゃありませんか」
名前は、なんていうか、状況に応じた適切な呼び方があるわけです。長ければ可読性が高いわけじゃないです。

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.

6 participants