Conversation
|
|
||
| ## フロイドの方法 | ||
| - サイクルの検知とサイクルの始点の検出を分けると良いというコメントを参考に整えた。 | ||
| - 最初はフラグ変数で実装していたが、小田さんのコメントで構造化が不十分と指摘されていた。そのため、典型コメント集の「コードの整え方」を参考に```while else```構文で実装した。 |
There was a problem hiding this comment.
次に続くものなどとの関係で他のものを使うこともあるので、解き進めながら時々思い出してください。
yas-2023
left a comment
yas-2023
left a comment
There was a problem hiding this comment.
お疲れ様です、変数名などが工夫された読みやすいコードでした。
2箇所だけコメントしております。
| - 最初はフラグ変数で実装していたが、小田さんのコメントで構造化が不十分と指摘されていた。そのため、典型コメント集の「コードの整え方」を参考に```while else```構文で実装した。 | ||
| - ループのネストをやめたので始点検出のための2つめのループで新しい変数名を使いたくなる気持ちになりやすくなった気がする。 | ||
| - step1のコードの読みにくさはループのネストだけでなく変数名の流用も影響していたと気が付いた。 | ||
| - 2つめのループでウサギ役の変数にどのような名前を付けるか迷ったが、無難にfrom_meetingとした。from_collisionにしようかとも思ったがネットワークの文脈で出てくる専門用語と名前がかぶって紛らわしいのでやめた。 |
There was a problem hiding this comment.
from_start, from_meetingという変数名はわかりやすいと思いました！
| return None | ||
| fast = head | ||
| slow = head | ||
| while fast.next is not None and fast.next.next is not None: |
There was a problem hiding this comment.
この部分ですが、
while fast is not None and fast.next is not None:
で良いと思うのですが、こうしなかった理由はなにかありますでしょうか？
上記とは別の観点のコメントとなりますが、
whileループ内のネストが浅くなるように工夫されていて良いと思いました。
There was a problem hiding this comment.
whileの条件については特に理由はなく、完全に私が見落としてただけです。
while fast is not None and fast.next is not None:
は簡潔ですし、冒頭のheadがNoneかどうかの判定も必要なくなるのでこっちの方がいいですね。
| return None | ||
| from_start = head | ||
| from_meeting = fast | ||
| while from_start is not from_meeting: |
There was a problem hiding this comment.
この分量だと迷うところですが「循環を検知する」や「循環の始点を検知する」をそれぞれ関数化してみてもいいかもしれません。
There was a problem hiding this comment.
ありがとうございます！step4で実装してみます！
| ## setを使った方法 | ||
| - 時間計算量: O(n), 空間計算量 O(n)。 | ||
| - 前回もらったコメントを参考に条件を整えた。 | ||
| - ループ条件は ```while node:```でもいいが、個人的に読みにくい気がしたので```is not```で判定した。 |
There was a problem hiding this comment.
とても良いと思います。
今回の問題の入力の制約からしてありえないということは抜きにすると、たとえばnodeが空リストだったりしてもwhile nodeはFalse判定されてしまいます。これが意図せぬ挙動を引き起こすこともあるため、Noneの確認をするときにはis not Noneと書いた方が安全だと思います
| # step2: 他の人のレビューを見てコードを整える | ||
| ## setを使った方法 | ||
| - 他の人は```set```オブジェクトの中身まで気にしていたが、自分は見落としていたのでドキュメントを調べた。 | ||
| - set型はハッシュ値を持つオブジェクトを格納するためのオブジェクトで、メンバーの判定```x in set```はハッシュ値を使ってオブジェクトの同一性をチェックしている。\ |
There was a problem hiding this comment.
めっちゃ細かいのですが、ハッシュでは同一性ではなく等価性をチェックしている、という方が一般的な気がします。
ハッシュ値が定義されているならば==演算子が定義されていることになりますが、==は等価性を確認するための演算子（という認識）であるからです、おそらく...
| return None | ||
| fast = head | ||
| slow = head | ||
| while fast.next is not None and fast.next.next is not None: |
There was a problem hiding this comment.
fast is None and fast.next is not None
と書いた方が個人的には好きです。移動できる限り移動していく、ということがそのまま条件になっているからです。
|
今更のレビューですみません... |
|
|
||
| ## フロイドの方法 | ||
| - サイクルの検知とサイクルの始点の検出を分けると良いというコメントを参考に整えた。 | ||
| - 最初はフラグ変数で実装していたが、小田さんのコメントで構造化が不十分と指摘されていた。そのため、典型コメント集の「コードの整え方」を参考に```while else```構文で実装した。 |
There was a problem hiding this comment.
すでにこちらにコメントをした通りですが、 while else 構文は避けたほうが無難だと思います。
MasukagamiHinata/Arai60#5 (comment)
6 participants
今回の問題: 142. Linked List Cycle II
次回の問題: 83. Remove Duplicates from Sorted List