Skip to content

Create Linked-List-Cycle.md#7

Open
riku359 wants to merge 2 commits into
mainfrom
Linked-List-Cycle
Open

Create Linked-List-Cycle.md#7
riku359 wants to merge 2 commits into
mainfrom
Linked-List-Cycle

Conversation

@riku359

@riku359 riku359 commented Dec 6, 2025

Copy link
Copy Markdown
Owner

@nodchip

nodchip commented Dec 6, 2025

Copy link
Copy Markdown

Pull Request のタイトルに問題番号と問題名を、 Description に問題へのリンクを張っておくと、レビューワーにとってレビューの助けになると思います。


visited = set()

current_address = 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.

Python でアドレスといった場合、 Python のインタープリターの内部でオブジェクトを格納しているメモリアドレスを指すように思います。 Python のコードからそれらを直接参照することはほとんどないため、変数名に現れるのは違和感があります。

また current という単語は、情報量が少ないように感じました。

自分であれば node といった、どのような値が格納されているかがより明確に伝わる名前を付けると思います。

ただ、previousnext と対比させたい意図がある場合には 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.

Linked Listのメモリアドレスを意識しての記述ですが、仰る通り変数にするのは読み手にとっては不親切ですね。
ご指摘ありがとうざいます。

@riku359 riku359 changed the title Step 1 Create Linked-List-Cycle.md Dec 6, 2025
@Yuto729

Yuto729 commented Dec 7, 2025

Copy link
Copy Markdown

読みやすく、よく理解されていて良いと思います!

current_address = current_address.next

# NoneになればCycleを抜けたことになるのでFalseを返す
return False

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

行末は改行を入れたほうがいいと思います。仮にこのコードをプログラムで処理するときに一貫性がなくなってしまうのと、diffなどを行ったときに新しいコードの部分だけでなく改行を入れた部分も変更として捉えられてしまって読みにくくなってしまうことが起こり得ると思います。
https://qiita.com/hamacccccchan/items/11c17c7412a5aeb2ad74
https://qiita.com/ko1nksm/items/3aea7fb8ac7e0396c10f

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.

Insert Final NewlineをONにしました。ご指摘ありがとうございます!

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