Conversation
canisterism
force-pushed
the
141_linked_list_cycle
branch
from
e860962 to
f5b72da
Compare
| - セルフツッコミしてみる | ||
| - もうちょっと最初の5行ぐらいをシュッと書けるかも | ||
| - 先にhead.valをaddするんじゃなくて、そのまま全部whileの中に入れられないかな | ||
| - `has_visited = visited_nodes.add?(current.val).nil?`が読みづらい気がする |
There was a problem hiding this comment.
includeメソッド使う方が素直な気がします。
https://docs.ruby-lang.org/ja/latest/class/Set.html#I_--3D--3D--3D
There was a problem hiding this comment.
確かに！ドキュメント途中までしか見てませんでした 😇
| return false if head.nil? || head.next.nil? | ||
|
|
||
| visited_nodes = Set.new | ||
| visited_nodes.add(head.val) |
There was a problem hiding this comment.
私もStep3のコードのように、headを特別扱いしないバージョンの方が好みです。
|
currentという命名についていろいろ議論されているので |
| - Step1はwhileの条件が読みにくいかも？改善ポイントってこういうのでいいのかな？ | ||
| - わからなかったので他の人のPRを探す | ||
| - https://github.com/sendahuang14/leetcode/pull/1/files | ||
| - HashMapでたどったノードをメモする解法。チラッと考えたけど前に見たことある解法で解くイメージが先行して試してなかった。 |
There was a problem hiding this comment.
講師陣の方によると、setを使った解法の方がオーソドックスというか想定解だと思われますね。
tk-hirom/Arai60#1 (comment)
| - そういえば計算量について全然意識できてなかったことに気づく | ||
| - 今のアルゴリズムだとどうなるんだ...？ | ||
| - 時間計算量は最大でO(N)？ | ||
| - 空間計算量はO(1)でいいのかな |
There was a problem hiding this comment.
合っていると思います。計算量を見ると、空間計算量がフロイドの解法の方がO(1)で良いですね。setを使うと最悪計算量がO(N)になるかと思います。(すべてのノードの値をsetに入れることになるため。)
|
コード自体は問題ないかと思いました。 |
| fast = fast.next.next | ||
| end | ||
| end | ||
| ``` |
There was a problem hiding this comment.
このコードで一番謎なのは、nil チェックするのと動かすのが対になっていないところでどう考えているのかが知りたいです。動かす前に確認したいし、確認したら動かしたい気持ちになりませんか。
There was a problem hiding this comment.
https://discord.com/channels/1084280443945353267/1192728121644945439/1194203372115464272
動かせるか確認してから、動かさない
ニンジンを切って、炊飯器のスイッチをいれて、ジャガイモを切れ、みたいな指示書かないじゃないですか。
There was a problem hiding this comment.
言われてみれば確かにそうですね。
これを書いた時の気持ちとして、「まず循環してるかの結果が出てたらすぐに判定してループを抜けたい」が先に来てif節を書いてました。
2歩動かせるか確認して動かせなかったらループはない。動かせるなら動かす。同じところにきたらループがあったと宣言
解いてる時にこの順番の発想が浮かびませんでした！ちょっと書き直してみます 👀
| return false if head.nil? || head.val.nil? | ||
| slow, fast = head, head | ||
|
|
||
| while !fast.nil? && !fast.next.nil? # <- fastがnilで落ちる |
There was a problem hiding this comment.
やってみました！仰ってることが肌で分かった気がします。
フロイドの循環検出法をloop+ifで書く
| current = head.next | ||
|
|
||
| while !current.nil? do | ||
| has_visited = visited_nodes.add?(current.val).nil? |
There was a problem hiding this comment.
これ、val が unique であるという保証あるんでしたっけ? Set は Object 入れられたと思いますので、そちらのほうが素直では。
There was a problem hiding this comment.
確認したのですが、勝手にvalがuniqueだと思い込んでました😇
（leetcode上のサンプルのテストケースは通ってたけどsubmitしたら他のケースで通ってませんでした）
Setにはプリミティブなintとかstringぐらいしか入らないと思い込んでたんですがHashも入るんですね。
ありがとうございます！
There was a problem hiding this comment.
https://docs.ruby-lang.org/ja/latest/library/set.html
Set は内部記憶として Hash を使うため、集合要素の等価性は Object#eql? と Object#hash を用いて判断されます。したがって、集合の各要素には、これらのメソッドが適切に定義されている必要があります。
There was a problem hiding this comment.
arai60/141_linked_list_cycle.md
Outdated
|
|
||
| while !current.nil? do | ||
| has_visited = visited_nodes.add?(current.val).nil? | ||
| has_visited = visited_nodes.add?(current).nil? |
There was a problem hiding this comment.
既存のを修正するより新しいSTEPで追加してもらうほうがレビューしやすいかなと思います。
There was a problem hiding this comment.
あああすみません、Step4で追記しました
arai60/141_linked_list_cycle.md
Outdated
| end | ||
| ``` | ||
| - 以下Odaさんのレビューを受けた時のログ | ||
| - valはすべてのノードでuniqueでないので、Hash自体を詰めないと同じvalが出てくると正しい結果にならない。↓ |
6 participants
https://leetcode.com/problems/linked-list-cycle/