141 linked list cycle#2
Conversation
| ## step3(10分以内にさっとかける * 3回) | ||
|
|
||
| 個人的に書いてて「自然」に感じられるのは set 使う方、という感想。 | ||
| Floyd の方は「へーこんなやり方が」って感じな一方 set 使う方は「まあそうだよな」って感じで、おそらく後者の方がいい場面が多いので。 |
There was a problem hiding this comment.
後者のほうが読みやすい気持ちが私もあります。
メモリ要件が入ってくるとFloydが選択肢になりそうでしょうか。
There was a problem hiding this comment.
ですね。リストがめちゃくちゃ長い時は Floyd のアルゴリズムが活きそうです。
| - Python3 は末尾再帰最適化が効かないっぽい? | ||
| - [他の方のコード](https://github.com/shintaro1993/arai60/pull/3/changes#diff-491120a14a1981900662359b52a1c34fca42db491aff99fb79c9ba09193ba12fR109) | ||
| - fast が先に行ってくれており slow の next は None じゃないことが保証されているので `slow is not None` は書かなくても正しく動きはする | ||
| - ただしこれだと Mypy(型チェッカー) は赤線引いてくる(たぶん単純な型推論だとわからなくて、コードの内容くわしく見ないといけないから) |
There was a problem hiding this comment.
ここをどう書くかは難しいと感じました。
(mypy全然知らないのですが、)自分なら型チェックを無視するコメントを残して、slow is not Noneは書かない方針にするかもしれません。
slow is not Noneがないと動かないと誤認する可能性を消すことを優先したい気持ちです。
型チェックのために余計な処理を書くことは本末転倒かなと。
There was a problem hiding this comment.
step4 書いてみたんですが、 while ループの冒頭で assert しておくのが型検査も満足させながら人間的にも意味が分かりやすくて良かったりしますかね?
例えば Google のスタイルガイドだと以下のように書かれてます。
Do not use assert statements in place of conditionals or validating preconditions. They must not be critical to the application logic. A litmus test would be that the assert could be removed without breaking the code. assert conditionals are not guaranteed to be evaluated. For pytest based tests, assert is okay and expected to verify expectations.
今回はもともと常に成り立つ条件を型検査機に教えてあげるコメントのような目的であり "the assert could be removed without breaking the code" に当てはまるので書いても良さそう、という感じです。(無視するコメントより情報量が多いという利点もありそうです)
There was a problem hiding this comment.
step4のコードもスタイルガイドもありがとうございます!
なるほど、こういうときはBigTechのスタイルガイドを参照すると参考になりますね。
勉強になりましたmm
共有していただいたスタイルガイドを読むと、Noの例で以下のコードが紹介されており、型チェックのためのassertはスタイルガイドでも推奨されていないかもしれません。
assert port is not None
# The type checking of the return statement relies on the assert.
return portまた、以下によると型チェックを無視することは選択肢として認める方針ではあるようです。
https://google.github.io/styleguide/pyguide.html#3197-ignoring-types
2.4.4と3.19.7しか読んでいませんが、型を守るためだけにプロダクションのコードに手を加えることは避けたほうが良いぐらいの感覚なのかなと思いました。
以下の観点おっしゃるとおりだなと思いました。
付随するコメントもコードと同じく人間に意味が伝わることが大事ですねmm
最終的には自分なら型を無視するコメントとなぜ無視できるかの理由のコメントを残しそうと感じました
人間的にも意味が分かりやすくて良かったりしますかね?
There was a problem hiding this comment.
具体例のところ見落としてました、、
「assert に依存していないか」というのには型チェックまで含むということなんですね。
注意深く読んでなくて勘違いしていましたが、型チェックを無視+その理由をコメントするという最初に仰っていたものが今回は一番しっくりきそうですね。(あるいは、冗長なのを承知で slow is not None を while の条件に含めてしまうか)
ありがとうございます!
| - Python では再起が好まれない話 | ||
| - デフォルトで深さが1000 |
| 参考になったコード・コメント・ドキュメント | ||
|
|
||
| - [他の方のメモ](https://github.com/naoto-iwase/leetcode/pull/1/changes/BASE..21cd6c5ca124e1b81b91b8003681e1a49b1c5927#diff-bb5a355bc7de70630b57c0d17d649e9adce45b5f0965089f22bf15dd4236d47fR75) | ||
| - [公式ドキュメント](https://docs.python.org/ja/3.13/library/stdtypes.html#comparisons) に「あるクラスの同一でないインスタンスは、通常等価でないとされますが、そのクラスが __eq__() メソッドを定義している場合は除きます。」「is および is not 演算子の振る舞いはカスタマイズできません。また、これらはいかなる 2 つのオブジェクトにも適用でき、決して例外を送出しません。」とあり、`==` は「(勝手に定義した)何らかの意味」で等しいというふうにできるのでオブジェクトが同一か知りたい今回は `is` を使うべき |
There was a problem hiding this comment.
Javaでも等値と等価という似たような話があったので、これは結構気をつけないと駄目なところだなぁと感じています。
| slow = head | ||
| fast = head |
There was a problem hiding this comment.
どうでも良い好みなのですが、fastが先でslowが後というアルゴリズムでfastの方がNoneチェックやslowとの比較でアルゴリズムの軸になっている感があるので、fast, slowの定義順だとwhile文を1ループ目で読むときなど色々スッキリ頭に入って着やすい気がしました。
(自分が書いていたときは全く気にしなかったのですが、人のコードを読むときになりました。不思議ですね。)
There was a problem hiding this comment.
確かにそうですね。
(自分が書いていたときは全く気にしなかったのですが、人のコードを読むときになりました。不思議ですね。)
私も言われてみればそうかもしれません
| visited_nodes.add(current_node) | ||
| current_node = current_node.next | ||
|
|
||
| if current_node in visited_nodes: | ||
| return True |
There was a problem hiding this comment.
個人的な処理のイメージは以下の順序でした。
その点で処理の順序にやや違和感があるのと、cycleなしのListの最後でcurrent_node.nextがNoneの場合にvisited_nodesにNoneがあるかチェックする少し無駄な処理が入る点でも先にcurrentがvisitedに含まれているか確認する方がきれいな気がしました。
- まずcurrentがvisitedにふくまれているか確認する
- 含まれていればTrueを返す
- 含まれていなければvisitedにcurrentを追加する
- currentを次のNodeに更新する
There was a problem hiding this comment.
と書きつつ、自分が解いたときはMA-yo-TA sanと同じ書き方をしたのでなんでだろうと振り返ると、おそらく最初の数要素分のループを実際に回すシミュレーションをしたときに、1つめの要素でvisited_nodesが空だとわかっているのに存在チェックするのが気持ち悪くて、nextの存在チェックをする書き方になるのかなぁという気持ちになりました。
好みだとは思いますが、アルゴリズム的には存在チェック→SetにNode追加→次のNodeに移動が自然な気が今はしています。
There was a problem hiding this comment.
最初の数要素分のループを実際に回すシミュレーションをしたときに、1つめの要素でvisited_nodesが空だとわかっているのに存在チェックするのが気持ち悪くて
自分も同じでしたが、言葉で説明するときは
チェック→SetにNode追加→次のNodeに移動
なので素直に書いた方がいいかもしれないですね。
この問題:https://leetcode.com/problems/linked-list-cycle/
次の問題:https://leetcode.com/problems/linked-list-cycle-ii/