Conversation
| - 'is' はオブジェクトの同一性を判定するための演算子。 | ||
| - '==' は値の比較のための演算子で、x == y は x.__eq__(y) という特殊メソッドを呼び出している。 | ||
| - __eq__() はデフォルトだと is と同じだが、カスタマイズ可能。 | ||
| - シングルトンが何なのかよく分からなかったが、絶対に1個のインスタンスしか持たないオブジェクトみたいなものだと理解しておく。 |
There was a problem hiding this comment.
pythonのドキュメントで解決しなかったのでwikipediaなどを参考にしました。ただ、ひらがなの読み書きを練習している段階で真剣に取り組むべき話題でもなさそうなのでアドバイス通り先に進むことにします。
https://ja.wikipedia.org/wiki/Singleton_%E3%83%91%E3%82%BF%E3%83%BC%E3%83%B3
yas-2023
left a comment
yas-2023
left a comment
There was a problem hiding this comment.
お疲れ様です、読みやすいコードでした！
2箇所だけコメントさせていただきました。
| - '==' は値の比較のための演算子で、x == y は x.__eq__(y) という特殊メソッドを呼び出している。 | ||
| - __eq__() はデフォルトだと is と同じだが、カスタマイズ可能。 | ||
| - シングルトンが何なのかよく分からなかったが、絶対に1個のインスタンスしか持たないオブジェクトみたいなものだと理解しておく。 | ||
| - 結局、Noneオブジェクトには絶対に1つしかインスタンスがないんだからオブジェクトの同一性をisで判定すれば十分、ということか。よくわからん。\ |
There was a problem hiding this comment.
この問題で==よりisが推奨されることが多いのは、
自分以外の読み手の負荷を下げるためと理解しています。
今回、自作（leetcode運営側の定義ではありますが）のオブジェクトを対象としているので、
==を使う場合、どのような実装になっているかを気にする必要がありますが
isだとそのような気遣いが読み手に不要になります。
今回のような小規模なコードでは大した問題ではないですが、
大規模なコードを多くのメンバで作成していく場合、
一人一人が読み手側の負荷を下げることが重要なのだと理解しています。
（私自身はまだ大規模プロジェクトに携わったことがないのですが）
There was a problem hiding this comment.
コメントありがとうございます。言語の仕様というよりは、人間側の認知不可を下げるためのルールだったのですね。私の場合、そもそもの開発経験がないのでなおのこと嬉しさが分かりにくかったのだと思います。
| def hasCycle(self, head: Optional[ListNode]) -> bool: | ||
| fast = head | ||
| slow = head | ||
| if fast is None: |
There was a problem hiding this comment.
if fast is None: の部分は、
fastを代入前の引数名headにして、
この行の場所もこの関数内の最初の行に移動すると、
より直接的に、引数headの不備で条件分岐するということがわかるため、読み手への負荷が下がるように思いました。
def hasCycle(self, head: Optional[ListNode]) -> bool:
if head is None:
return False
fast = head
slow = head
There was a problem hiding this comment.
確かにおっしゃる通りですね。指摘されるまで気づいていなかったので、コードの整理はかなり課題がありそうです。意識的に改善できるよう精進します。
| node = head | ||
| if node is None: | ||
| return False | ||
| while node.next is not None: |
There was a problem hiding this comment.
ループの中ではnode.next is not Noneであることを必要としないので単にnode is not Noneで良いかなと思いました。そうすると79-80行目の例外処理も要らなくなります。
There was a problem hiding this comment.
確かにおっしゃる通りですね。全然気が付いてなかったです。コードの条件の整理はかなり苦手のようなので改善できるよう精進します。
| if node in checked_nodes: | ||
| return True | ||
| checked_nodes.add(node) | ||
| node = node.next |
There was a problem hiding this comment.
while node is not None:
if node in checked_nodes:
return True
checked_nodes.add(node)
node = node.nextと書かなかった理由はありますか？
この問題において、とあるノードをチェックする条件がその次のノードが存在することであることがあまり直感的ではないような気がしました。
| slow = head | ||
| if fast is None: | ||
| return False | ||
| 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でなかった理由が気になります👀
|
すごく今更のレビューになってしまってすみません。 |
レビューありがとうございました。練習会の目的はエンジニアの常識を身につけることなので、コメントの重複はやむを得ないですし、むしろ重複したほうが常識的な反応ができているということなので望ましいのかなと思います。 |
5 participants
This problem: 141. Linked List Cycle
https://leetcode.com/problems/linked-list-cycle/description/
Next problem: 142. Linked List Cycle II
https://leetcode.com/problems/linked-list-cycle-ii/