Skip to content

142. Linked List Cycle II#2

Open
Zun-U wants to merge 2 commits intomasterfrom
142/linked-list-cycle-II
Open

142. Linked List Cycle II#2
Zun-U wants to merge 2 commits intomasterfrom
142/linked-list-cycle-II

Conversation

@Zun-U
Copy link
Copy Markdown
Owner

@Zun-U Zun-U commented Oct 13, 2025

```

新井浩平氏の解説動画を見ると、初見では理解できませんでした。
`fast`が2倍の速度で移動するのでサイクルを必ず1周以上すること、`slow`は絶対に1周しないことが分かると、理解が進みました。
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

レビューありがとうございます。
科学手品的な手法にとらわれないように心がけます。

Comment on lines +155 to +174
func detectCycle(head *ListNode) *ListNode {
fast := head
slow := head

for fast != nil && fast.Next != nil {
fast = fast.Next.Next
slow = slow.Next

if fast == slow {
slow = head
for fast != slow {
fast = fast.Next
slow = slow.Next
}
return fast
}
}

return nil
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

for-if-forとネストが深く、読みにくい印象を抱きます。
外側のforと内側のforは逐次実行できるので、分離してネストを浅くしたいですね。

例えば以下のように前半を関数化するのはどうでしょうか。

func hasCycle(head *ListNode) (bool, *ListNode) {
	fast := head
	slow := head
	for fast != nil && fast.Next != nil {
		fast = fast.Next.Next
		slow = slow.Next
		if fast == slow {
			return true, fast
		}
	}
	return false, nil
}

func detectCycle(head *ListNode) *ListNode {
	found, meetingNode := hasCycle(head)
	if !found {
		return nil
	}

	fromHead := head
	fromMeeting := meetingNode
	for fromHead != fromMeeting {
		fromHead = fromHead.Next
		fromMeeting = fromMeeting.Next
	}
	return fromHead
}

また、他にも書きかえ方はいくつかあり、oda-sanがまとめられた「コードの整え方」が大変参考になります。

https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.9kpbwslvv3yv

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

あ、先手を取られました。ありがとうございます。

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[head] = true
head = head.Next
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

head はリンクリストの先頭のノードを表す単語のため、 head が動いていくのに違和感を感じました。 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.

レビューありがとうございます。
headが「その時点の先頭」と勝手に自己解釈しておりました。
命名時にその意味をなるべくそのまま解釈して違和感がないように心がけます。

```Go
func detectCycle(head *ListNode) *ListNode {
visited := make(map[*ListNode]bool)
current := head
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

current という変数名にはあまり情報がないように思います。自分なら node と付けると思います。
previous/next と対比したいときには current と付けるのもありだと思います。

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.

リンクリストを、単一のnodeで表現してよいか悩んでおりました。
こちらシンプルに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.

4 participants