Skip to content

142. Linked List Cycle II #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

142. Linked List Cycle II #4

wants to merge 2 commits into from

Conversation

rinost081
Copy link
Owner

  1. Linked List Cycle II を解きました。
    問題: https://leetcode.com/problems/linked-list-cycle-ii/
    言語: Python

レビューをお願いします。


if fast == slow:
slow = head
while(fast != slow):
Copy link

Choose a reason for hiding this comment

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

while fast != slow:の方が適切だと思います。
Pythonでwhile/if文の条件式に括弧を用いるのは式が複雑で可読性を上げたい時だけで、基本的に括弧は用いないと思います。
https://google.github.io/styleguide/pyguide.html#33-parentheses

Copy link
Owner Author

Choose a reason for hiding this comment

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

全く気にしたことがなかったところです。ありがとうございます!

というか、141ではset()でやって、しかも最初の方針でもset()を使うようにと書いているのになぜか今回のケースだとできなくねと判断してdict{}でやってしまっている。他のデータ構造でも代用できたと、ポジティブに捉えることができるが計算も遅いし、実装としてはあまりよろしくない。
あ、これは問題を読み間違えていてnodeを返すのではなく位置を返すと勘違いしていることから起きたことですね。結局テストケースを試してみて位置を返すのではなくnodeを返すんじゃんと気づき、その場でdictをささっと修正して提出したのが原因なようです。落ち着きましょう。

dictとset()で解いた場合、それぞれで比較してみましたが速度の違いが断然違いますね。なんでかを調べます。
Copy link

@h1rosaka h1rosaka Sep 1, 2024

Choose a reason for hiding this comment

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

これ私もわからなかったので少し調べてみました。

このサイトに下記のように記載があり、確かに重複しうるからハッシュにできなくて実質リストみたいになってて一個ずつ確認するしかないからinが遅いのかな、と私は納得しました。ハッシュが速い理由はこのサイトがわかりやすいです。

The key of the dictionary is a unique value as well as the set, and the execution time is about the same as for sets.

On the other hand, dictionary values can be duplicated like a list. The execution time of in for values() is about the same as for lists.

Choose a reason for hiding this comment

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

これは、dictで解く際に
visited_nodes: dict[int, ListNode]
ではなくて
visited_nodes: dict[ListNode, int]
等のようにListNodeをdictのキーにして、if node in visited_nodes:とかで判定すると概ね同じぐらいの速さになります。↑のコメントに書かれているようにkeysとvaluesで構造が違っています。

Copy link
Owner Author

Choose a reason for hiding this comment

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

visited_nodes: dict[int, ListNode]の方がvisited_nodes: dict[ListNode, int]より速い理由について調べているのですが、中々良い考察を見つけられません。理由をご存知でしたら教えていただきたいです、、、。

Copy link

Choose a reason for hiding this comment

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

上記の議論はPythonの辞書のkeyとvalueの探索の違いについて書かれています。
辞書の中から任意のkeyを探索するのにかかる時間はO(1)です。なぜなら、Pythonの辞書はハッシュマップで実装されており、任意のkeyに対してハッシュ値を計算してその値を元にメモリ空間の中からこのkeyはここに格納されているはずと決め打ちできるからです。
一方、任意のvalueを探索しようと思うと、辞書の中に格納されているすべてのvalueを舐めるように探索するしか方法がないので、O(n)時間かかります。
つまり、辞書型はkeyの探索の方がvalueの探索より断然速いです。なので、visited_nodesの中から任意のListNodeを探索する必要があるのならListNodeをvalueではなくkeyに設定した方が良いということだと思います

Copy link
Owner Author

Choose a reason for hiding this comment

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

なるほどそういうことなのですね。ずっとintの方がListNodeよりも速いという理解をしていたのでその理由を考えていました。そうではなくて探索をするときにkeyを見るか、valueを見るかという話ということなのですね。ありがとうございます。

Copy link

Choose a reason for hiding this comment

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

ちなみに #4 (comment) で記載されている「より速い」の順序が逆になっています!

Copy link
Owner Author

Choose a reason for hiding this comment

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

タイポですね、、、指摘ありがとうございます!


class Solution:
def detectCycle(self, head: Optional[ListNode]) -> Optional[ListNode]:
visited_node = {}
Copy link

Choose a reason for hiding this comment

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

複数の値が格納される変数の変数名は複数形で終えたほうが、中に複数の値が含まれていることが分かりやすくなり、読んでいる人にとって読みやすく感じられると思います。


"""step2
141のコードレビューで指摘された、最初にアルゴリズムを言語化するというのはできたが、時間計算量と空間計算量を概算するという過程を忘れていたので、次回では忘れずにやりたい。
解答を見る感じだと141みたいにフロイドの循環アルゴリズムを使う方法があるっぽい。だけど理由がわからない。
Copy link

Choose a reason for hiding this comment

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

フロイドの循環アルゴリズムはソフトウェアエンジニアの常識には含まれていないと思います。 set() を用いた解法が書ければ十分だと思います。

Copy link
Owner Author

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

Choose a reason for hiding this comment

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

面接官から見ると、速い人と遅い人で速度が10倍違うので、速い人のために時間つなぎがしたくなることがあるんですよ。そういうときに、パズル要素があると、パズルを説明したりしていると時間が使えるので、そこが評価ポイントでなかったとしても便利なんです。また、受けている人を歓待したいという気持ちがあります。少なくとも、自社にいい印象を持っている人を怒らせて帰す必要はないですからね。転職が多い業界でいつ一緒に働くかは分からないわけですし。
一方で、それを体験した人はパズル部分が印象を残すので、そこが強く評価されたポイントだったのではないかと思う傾向があります。

https://github.com/h1rosaka/arai60/pull/4#discussion_r1722542513 プライベートメソッドだとアンダースコアをつけるのが普通なんですね。

というか、141ではset()でやって、しかも最初の方針でもset()を使うようにと書いているのになぜか今回のケースだとできなくねと判断してdict{}でやってしまっている。他のデータ構造でも代用できたと、ポジティブに捉えることができるが計算も遅いし、実装としてはあまりよろしくない。
あ、これは問題を読み間違えていてnodeを返すのではなく位置を返すと勘違いしていることから起きたことですね。結局テストケースを試してみて位置を返すのではなくnodeを返すんじゃんと気づき、その場でdictをささっと修正して提出したのが原因なようです。落ち着きましょう。

Choose a reason for hiding this comment

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

これは重要な気づきだと思います。
一般論として、「がんばって書いたコード」を書き上げた瞬間は、それまでの思考の経緯に引きずられて、変な構造や冗長な記述がコードに含まれています。
こうして思考の経緯を言語化すると、「posからnodeへのdictを元々作っていたため、valuesを使って解いてしまった(が、valuesはlistと同等の構造のため、inで探すのには適していない)」といった具合で整理できてとても良いですね。

Copy link
Owner Author

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.

主題と関係ないのですが、この問題に関しては問題文が少し理解しづらいので、気持ちはわかります。。。
面接ではこの辺を書き始める前に擦り合わせておくことは重要そうですね。


class Solution:
def detectCycle(self, head: Optional[ListNode]) -> Optional[ListNode]:
visited_node = set()
Copy link

Choose a reason for hiding this comment

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

nodesの方が良いかと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

複数のオブジェクトを扱うときは変数名も気をつけた方が良さそうですね、ありがとうございます

@rinost081 rinost081 changed the title Leet code142 142. Linked List Cycle II Dec 5, 2024
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.

7 participants