-
Notifications
You must be signed in to change notification settings - Fork 0
142 linked list cycle ii medium #20
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
base: main
Are you sure you want to change the base?
Conversation
…ing Floyd's algorithm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
分かりやすいです。
nanae772
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
お疲れ様です、全体的に読みやすいコードでした!
| ```python | ||
| class Solution: | ||
| def detectCycle(self, head: ListNode | None) -> ListNode | None: | ||
| if not head: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これでも問題無いと思うのですが、個人的にはif head is None:のほうが自然かなと思いました
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
falsy判定なので、明示あったほうが良いと思いました。
https://stackoverflow.com/questions/39983695/what-is-truthy-and-falsy-how-is-it-different-from-true-and-false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
レビューありがとうございます。
確かに、自作オブジェクトの場合、if head is None: ... と明示した方が良い気がしますね。
理由として考えたのは
- メソッドのオーバーライドで予期せずfalsy判定になると良くない。
- 一方で、空のListNodeはfalsyとなるように実装されている場合、
if head is None: ... elif head is EmptyNode: ...と条件分岐が二つになってしまうので、if not head: ...とかけた方が嬉しくなりそう
- 一方で、空のListNodeはfalsyとなるように実装されている場合、
- 予期せぬfalsyなオブジェクトが入ってきた時に、エラーにならずコードが動いてしまう可能性がある
(もしbuilt-inのオブジェクトなら、タプルでもリストでもsetでも良いようにif not obj: ... と書きたくなりますが)
| while fast_pointer and fast_pointer.next: | ||
| slow_pointer = slow_pointer.next | ||
| fast_pointer = fast_pointer.next.next | ||
| if slow_pointer == fast_pointer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここでは値の等価性よりオブジェクトの同一性を見たいと思うのでisを使うほうが自然な気がしました。
ただ上で書かれているように自作クラスのデフォルトでは==はidでの判定になり、isと同じ結果になるのでそれを承知の上でこうされているのなら問題ありません。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
自作オブジェクトの記述を見に行くのがめんどうなので、個人的にはisの方がありがたいです
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
レビューありがとうございます。
確かに、自作オブジェクトの__eq__メソッドがオーバーライドされているかどうかに依存するのは良くないですね。例えばListNode を継承する子クラスだけ書き換えていた場合、予期せぬ挙動の元になりますね。isが適切だと感じました。
| """ | ||
| slow_pointer = head | ||
| fast_pointer = head | ||
| have_cycle = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
動詞は三人称単数形にするのが一般的のようなので、has_cycleかなと思いました
| pointer1 = head | ||
| pointer2 = fast_pointer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from_head, from_meetingなどの名前をつけてもいいかなと思いましたが、スコープが短いのでここはこれでもよいかもしれません
| 2. もし`slow`と`fast`が衝突した場合、サイクルが存在する。 | ||
| 3. サイクルの開始ノードを特定するために、`slow`をリストの先頭に戻し、`fast`はそのままの位置に置く。両方を1ステップずつ進めて次に衝突するノードがサイクルの開始ノードである。 | ||
|
|
||
| - ループが`break`されず。正常に終了したかどうかを表す`for ... else`はEffective Pythonではバッドプラクティスとされているため、`exists_cycle`フラグを使用している。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for-elseはバッドプラクティスなのですね。確かに他の言語には無く、またあまり見かけないので実際目にすると「elseに行くのはどういう条件だっけ?」と悩むことになるので使わないほうがよいというのは納得です。
勉強になりました!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
たた、フラグは goto よりも構造化されていないと思いますね。
コードの整え方のあたりを見ておいてください。
https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.9kpbwslvv3yv
| ``` | ||
|
|
||
| `step3_two_pointers.py` | ||
| やはりフロイドのアルゴリズムは読んでいてわかりにくいので、docstringを追加してみた。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docstringいいですね、親切だと思いました👍
|
全体的に見やすくていいと思います。 |
| - もし`__eq__`がオーバーライドされている場合、`==`は意図しない動作をする可能性がある | ||
| - `None`チェックは`if not head:`よりも、`if head is None:`の方が今は明示的で良い | ||
| - ここでもメソッドのオーバーライドで予期せずfalsy判定になると良くない。 | ||
| - 一方で、空のListNodeはfalsyとなるように実装されている場合、`if head is None: ... elif head is EmptyNode: ...` と条件分岐が二つになってしまうので、`if not head: ...` とかけた方が嬉しくなりそう |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これは確かにそうですね。
私は今回headがNoneになっている場合が空の連結リストを表している(ListNodeが入ってきた時点で1つはノードがある→空ではない)と判断していたのでhead is Noneの判定がいいかなと思っていました。
ですが懸念されているように空のリストを表すListNodeみたいなものが決められている場合もありそうですね。
勉強になりました、詳しく書いていただきありがとうございます!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
良かったです。こちらこそレビューありがとうございました。
|
読みやすかったです。 |
問題へのリンク
142. Linked List Cycle II
言語
Python
問題の概要
与えられた連結リストがサイクルを持つ場合、そのサイクルの開始ノードを返す。サイクルがない場合は
Noneを返す。自分の解法
step1
visitedセットを用いて到達済みノードを記録する方法。自作オブジェクトはhashableなので、setやdictのキーとして使用できる
__hash__メソッドがhash(id(self))を返すため、オブジェクトのIDに基づいてハッシュ値が生成される__eq__メソッドはid(self) == id(other)を返すため、オブジェクトのIDに基づいて等価性が判断される__eq__メソッドが定義されている場合、__hash__メソッドも適切に定義する必要があるa==bがTrueの場合、hash(a)とhash(b)も等しくなる必要がある時間計算量:
O(n)空間計算量:
O(n)step2
step3
step3.pystep3_two_pointers.pyやはりフロイドのアルゴリズムは読んでいてわかりにくいので、docstringを追加してみた。
step4 (FB)
別解・模範解答
フロイドのアルゴリズムを使用する方法。
まずは、2つのポインタを用いてサイクルの存在を検出する。次に、サイクルの開始ノードを特定する。
slowとfast)を用意し、slowは1ステップずつ、fastは2ステップずつ進める。slowとfastが衝突した場合、サイクルが存在する。slowをリストの先頭に戻し、fastはそのままの位置に置く。両方を1ステップずつ進めて次に衝突するノードがサイクルの開始ノードである。ループが
breakされず。正常に終了したかどうかを表すfor ... elseはEffective Pythonではバッドプラクティスとされているため、exists_cycleフラグを使用している。時間計算量:
O(n)空間計算量:
O(1)次に解く問題の予告