Skip to content

141. Linked List Cycle #12

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

Merged
merged 2 commits into from
Jan 30, 2025
Merged

141. Linked List Cycle #12

merged 2 commits into from
Jan 30, 2025

Conversation

katsukii
Copy link
Owner

### 解法1. フロイドの循環検出法
* 1つ飛ばしで進むウサギ(fast)と1つずつ進む亀(slow)の2つのポインタを同時に同じリスト上を走らせると、サイクルがある場合必ずどこかのノードで合流するという考え方
* https://en.wikipedia.org/wiki/Cycle_detection#Floyd's_tortoise_and_hare
* 感想: 2つ走らせるという発想はなかった。時間をかけても自分でゼロから思いつくのは厳しいと思う。考え方のパターンとしてストックしたい
Copy link

Choose a reason for hiding this comment

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

このあたりは、科学手品みたいなものなので、あまり気にしなくてよいかと思います。
https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.2k4z0wt6ytf9

Copy link
Owner Author

Choose a reason for hiding this comment

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

なるほど、すべてを真面目に捉えてすぎていたので、もう少し気楽に考えてみます。ありがとうございます。

```java
public class Solution {
public boolean hasCycle(ListNode head) {
ListNode current = head;
Copy link

Choose a reason for hiding this comment

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

currentという変数名の是非はdiscordで時々話題に上がっているので見てみてください

Copy link
Owner Author

Choose a reason for hiding this comment

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

調べたら大量に出てきました。。勉強になりました。
今回はnodeに変更しようと思います。

atomina1/Arai60_review#2 (comment)

current という単語は、 previous や next と対比するのであれば意味があると思いますが、単独で使ってもあまり情報がないように思います。 node はいかがでしょうか?

https://discord.com/channels/1084280443945353267/1231966485610758196/1236231890038689844

これくらいの長さだったら、まあいいとは思うんですが、本当は current という名前は、あんまり情報量がないので、もう少し情報の載ったもののほうがいいかもしれません。
(current: 現在注目しているもの、くらいのつもりですよね。)

katataku/leetcode#7 (comment)

なんか全体的に命名が分かりづらいなと思いました。

current_headのcurrentには現在注目しているくらいの意味しかなくて、reversed_headとどう違うんだろうみたいな感想を持ちました。あとは、reversed_headとありますが、current_headもreverseしてるじゃんみたいな気持ちになりました。

public class Solution {
public boolean hasCycle(ListNode head) {
ListNode current = head;
HashSet<ListNode> visitedSet = new HashSet<>();
Copy link

Choose a reason for hiding this comment

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

javaがどうかはわかりませんが、pythonでは変数名に型名を入れるのを避けるべきだみたいな話があります。

javaに関しても、型名が変数名に入っていても読みやすさは変わらないように思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

Javaでも一般的に型名を入れない方がいいみたいですね。修正します。ありがとうございます。

Comment on lines +75 to +79
```java
public class Solution {
public boolean hasCycle(ListNode head) {
ListNode node = head;
HashSet<ListNode> visitedNodes = new HashSet<>();

Choose a reason for hiding this comment

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

レビュー遅くなりすみません。

全体的に良いと思います!ただすでに指摘されているように変数名が気になりました。visitedNodesのように変数名に大文字を含むのはpepでは推奨されていないようです。
そのためvisited_nodesなどのようにするほうがいいと思いますが、そもそもnodeが入っていないときもあるので、僕だったらvisitedくらいにします。

関数や変数の名前
関数の名前は小文字のみにすべきです。また、読みやすくするために、必要に応じて単語をアンダースコアで区切るべきです。
変数の名前についても、関数と同じ規約に従います。
mixedCase が既に使われている (例: threading.py) 場合にのみ、互換性を保つために mixedCase を許可します。

https://pep8-ja.readthedocs.io/ja/latest/#id35

Choose a reason for hiding this comment

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

おそらく別のドキュメントですが、仰る通り同じく、PEP8 という規約では変数名と関数名は lower_case_with_underscores を使いましょうとなっているようです。
https://peps.python.org/pep-0008/#descriptive-naming-styles

Copy link
Owner Author

Choose a reason for hiding this comment

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

@lilnoahhh @olsen-blue
レビューありがとうございます。たしかにvisitedの方が良さそうですね。

変数名の命名規則に関してですが、PEP8はPython向けの規約のため一概にJavaのコードに適用すべきではないと考えています。以下はGoogleのコーディング規約ですが、JavaではlowerCamelCaseが推奨されているようです。

Local variable names are written in lowerCamelCase

https://google.github.io/styleguide/javaguide.html#s5.2.7-local-variable-names

Choose a reason for hiding this comment

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

その通りですね、失礼しました

* 1つ飛ばしで進むウサギ(fast)と1つずつ進む亀(slow)の2つのポインタを同時に同じリスト上を走らせると、サイクルがある場合必ずどこかのノードで合流するという考え方
* https://en.wikipedia.org/wiki/Cycle_detection#Floyd's_tortoise_and_hare
* 感想: 2つ走らせるという発想はなかった。時間をかけても自分でゼロから思いつくのは厳しいと思う。考え方のパターンとしてストックしたい
* [追記]レビュアーからのコメントとして、この解法は科学手品のようなものなので自分で思いつけなくてもあまり気にしなくてよいとのこと。一方でStep1のSetを使った解法は思いつけて普通とのこと

Choose a reason for hiding this comment

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

全体的に良いと思います。Discord内に直感的な解説も転がってるので、まだ見てなければどうぞ。
https://discord.com/channels/1084280443945353267/1246383603122966570/1252209488815984710

Copy link
Owner Author

Choose a reason for hiding this comment

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

非常に分かりやすいですね。とても参考になりました。
ありがとうございます。

@katsukii katsukii merged commit 06d111e into main Jan 30, 2025
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