Skip to content

2. Add Two Numbers#7

Open
xbam326 wants to merge 3 commits into
mainfrom
2
Open

2. Add Two Numbers#7
xbam326 wants to merge 3 commits into
mainfrom
2

Conversation

@xbam326
Copy link
Copy Markdown
Owner

@xbam326 xbam326 commented Dec 30, 2025

fixed_node = dummy
carry = 0

n1, n2 = l1, l2
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

私は「引数は変更しない」ことをどこかで教わってきたのですが、必ずしもそうではなさそうです。例えば
DaisukeKikukawa/LeetCode_arai60#5 (comment) の議論を参照ください。

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.

ありがとうございます
確認したところmamo3grさんと同じ理由で同じスタンスだったのですが、他の方にも変更されても気にしないスタンスの方がいることは認識しておこうと思いました。
現状の理解だと引数の中身を変更を変更することが目的の関数以外で引数の中身を変更した方が好ましいようなことがなければ、無理に今のスタンスを変えて合わせにいかなくてもいいのかなと考えております。

self, l1: Optional[ListNode], l2: Optional[ListNode]
) -> Optional[ListNode]:
dummy = ListNode()
fixed_node = dummy
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[nits]
ここではnodeが具体的に桁 (digit) を表しているので、そのような具体的な命名がより親切に思いました。

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.

ありがとうございます
確かに教えていただいた方が何をやっているかイメージが湧きやすく優れていると思いました。

Comment on lines +17 to +18
dummy = ListNode()
fixed_node = dummy
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

同じ値を複数の変数に定義する場合は一行にまとめることが可能です。
fixed_node = dummy = ListNode()

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.

ご指摘ありがとうございます。
確かにスッキリ書けるので今後そちらを使っていこうと思います。


total = val1 + val2 + carry
carry, fixed_node_val = divmod(total, 10)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

私も10を使ってしまっているんですが、マジックナンバーになりそうですね。
定数化して名前をつけた方がよさそうです。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

私は10が10進法の10であるというのは通じると思うのでこれでもいいかと思います。
後から変更したくなった時に10が散らかっていると大変ではあります。

) -> Optional[ListNode]:
dummy = ListNode()
fixed_node = dummy
hasCarry = 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.

変数名に lower_snake と lowerCamel が混ざっているのが気になりました。 lower_snake で統一することをお勧めいたします。

参考までにスタイルガイドへのリンクを共有いたします。

https://peps.python.org/pep-0008/#function-and-variable-names

mixedCase is allowed only in contexts where that’s already the prevailing style (e.g. threading.py), to retain backwards compatibility.

なお、このスタイルガイドは“唯一の正解”というわけではなく、数あるガイドラインの一つに過ぎません。チームによって重視される書き方や慣習も異なります。そのため、ご自身の中に基準を持ちつつも、最終的にはチームの一般的な書き方に合わせることをお勧めします。

) -> Optional[ListNode]:
dummy = ListNode()
fixed_node = dummy
hasCarry = 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.

識別子が動詞の命令形もしくは三単現で始まっていると、関数名のように感じます。 step2 で既に修正されている通り、 carry で十分に伝わると思います。

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.

5 participants