Skip to content

Reverse Linked List #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Reverse Linked List #8

wants to merge 1 commit into from

Conversation

bumbuboon
Copy link
Owner

```python
class Solution:
def reverseList(self, head: Optional[ListNode]) -> Optional[ListNode]:
if head is None:

Choose a reason for hiding this comment

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

headを特別扱いしなくても良さそうです

Copy link
Owner Author

Choose a reason for hiding this comment

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

確かにその通りです。
あまり考えずにとりあえずという感じで書いていました。

```python
class Solution:
def reverseList(self, head: Optional[ListNode]) -> Optional[ListNode]:
reversed = None

Choose a reason for hiding this comment

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

reversedはbuiltin functionにあるので命名としては避けるのが無難かなと思います。
https://docs.python.org/3/library/functions.html#reversed

Copy link
Owner Author

@bumbuboon bumbuboon Oct 24, 2024

Choose a reason for hiding this comment

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

biltinは関数ですが僕が定義したのは変数なので使っても良いのではないかと思って使ってしまいました。
reversed_でしたら大丈夫でしょうか?

Choose a reason for hiding this comment

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

これとか実行してみると良いと思います。

print(type(reversed))
reversed = 5
print(type(reversed))

Choose a reason for hiding this comment

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

あまり詳しくないですが、local name spaceにおいてはreversedも上書きされてしまうっぽいです
https://docs.python.org/3/glossary.html#term-namespace

Copy link

Choose a reason for hiding this comment

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

プログラムが動くか動かないかという意味で大丈夫か大丈夫ではないかというと大丈夫です。keyword はだめです。(keyword 一覧確認しておきましょう。)

ただ、builtin も後々の書き換えでそれを使いたい人が現れると混乱させるので、書き慣れた人は避けるでしょう。(builtin 一覧も確認しておきましょう。)

Copy link
Owner Author

Choose a reason for hiding this comment

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

なるほど、ありがとうございます。確認してみたいと思います。

Copy link

Choose a reason for hiding this comment

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

公式ドキュメントを探して下のように貼り付けるまでを、習慣にするといいでしょう。
https://docs.python.org/3/reference/lexical_analysis.html#keywords
https://docs.python.org/3/library/builtins.html#module-builtins

専門家の振る舞いとして、読んで調べるのは大事です。

Comment on lines +164 to +169
while not_reversed:
next_temp = not_reversed.next
not_reversed.next = reversed

reversed = not_reversed
not_reversed = next_temp

Choose a reason for hiding this comment

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

next_tempが思いつきづらいとありましたが、
ここは書こうと思えば、
not_reversed.next, reversed, not_reversed = reversed, not_reversed, not_reversed.next
みたいにnext_tempを使わずに1行で書けます。
たださすがに可読性が悪すぎるので、一時的にnext_tempに退避させていると考えたら
すっと思いつけるのではないかと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

その考え方はなかったのでアドバイス頂けてありがたいです。

stack.append(node.val)
node = node.next

dummy = ListNode(-1)

Choose a reason for hiding this comment

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

スタックから初期ノードを持ってくれば、ダミーが不要になってコードがシンプルになりそうですね

Copy link
Owner Author

Choose a reason for hiding this comment

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

確かにそうですね、思いつきませんでした。ありがとうございます。

if head is None:
return None

stack = list()
Copy link

@Mike0121 Mike0121 Oct 26, 2024

Choose a reason for hiding this comment

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

質問ですが、listを[]ではなく、list()として定義した意図はありますか?

自分も気になって調べましたが、
stack = []の方が関数呼び出しが行われない分自然みたいです。
https://stackoverflow.com/questions/33716401/whats-the-difference-between-list-and

Copy link

Choose a reason for hiding this comment

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

たしか辞書の初期化も dict() より = {} の方が推奨と聞いたことがあります
https://stackoverflow.com/questions/17097985/dict-vs-in-python-which-is-better
https://switowski.com/blog/dict-function-vs-literal-syntax/

class Solution:
# leetcodeはデフォルトの再帰の深さの最大数が55,000のため、今回の制約(最大5,000)だとRecursionErrorは考慮しなくて良い
def reverseList(self, head: Optional[ListNode]) -> Optional[ListNode]:
def _reverse_linked_list(head, previous):
Copy link

Choose a reason for hiding this comment

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

inner function の名前を _ で始めることは少ないように思います。

_ で始めるのは、メンバ関数のうち non-public なものが多いと思います。
https://peps.python.org/pep-0008/#method-names-and-instance-variables

Use one leading underscore only for non-public methods and instance variables.

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.

8 participants