Skip to content

Add 2. Add Two Numbers.md #5

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 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
243 changes: 243 additions & 0 deletions 2. Add Two Numbers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
# step 1
下の桁から足していく足し算をする問題
```python3
class Solution:
def addTwoNumbers(self, l1: Optional[ListNode], l2: Optional[ListNode]) -> Optional[ListNode]:
node1 = l1
node2 = l2
dummy_head = ListNode()
sum_node = dummy_head
carry = 0
while node1 is not None or node2 is not None:
value1 = 0
value2 = 0
if node1 is not None:
value1 = node1.val
node1 = node1.next
if node2 is not None:
value2 = node2.val
node2 = node2.next
Copy link

@ichika0615 ichika0615 Nov 26, 2024

Choose a reason for hiding this comment

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

この部分がわかりづらく思いました。所与の連結リストを走査していき、ノードがあるならvalue1ないしはvalue2にその数字を格納して、ノードがすでに終わっているなら加算するものがないから0を代入する、というロジックかと思います。

if node1 is not None:
    value1 = node1.val
else:
    value1 = 0

が一番しっくりくると思います。
また、合計を計算したりする前にノードを次に進めているのもわかりづらく感じました。


sum_value = (value1 + value2 + carry) % 10
carry = (value1 + value2 + carry) // 10
Comment on lines +12 to +22

Choose a reason for hiding this comment

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

value1, value2 はともに一時変数だと思うので carry も含めて1つの変数で扱うのがわかりやすいのでは、と思いました。結局興味があるのは value1 + value2 + carry なので。(tmp_val がよい変数名かは微妙な気もしますが)

Suggested change
value1 = 0
value2 = 0
if node1 is not None:
value1 = node1.val
node1 = node1.next
if node2 is not None:
value2 = node2.val
node2 = node2.next
sum_value = (value1 + value2 + carry) % 10
carry = (value1 + value2 + carry) // 10
tmp_val = carry
if node1 is not None:
tmp_val += node1.val
node1 = node1.next
if node2 is not None:
tmp_val += node2.val
node2 = node2.next
carry, digit = divmod(tmp_val, 10)

sum_node.next = ListNode(val=sum_value)
sum_node = sum_node.next

if carry == 1:
sum_node.next = ListNode(val=1)
Comment on lines +26 to +27
Copy link

@thonda28 thonda28 Nov 26, 2024

Choose a reason for hiding this comment

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

この問題において carry には 0 または 1 しか入りませんが、ここで気にしていることは carry があるかないか(0 かそうでないか)かと思うので、if carry: または if carry > 0: という条件のほうが意図が伝わる気がします。

またこの問題では2つのノードですが、後からやっぱり3つのノードも扱えるよう拡張したりすると、(新たに2が入りうるため)この部分で予期せぬバグが生まれたりもしそうです。(実務だとそういった後からの拡張もありえそうです)


return dummy_head.next
```

修正可能箇所
- whileのすぐ下のvalue1, value2は三項演算子を使うと、行数短縮できる。
pythonだとTrue時の値が最初にくる()ので、読みにくいように思う。
- `node1`,`node2`という変数名に対して`sum_node`とnodeという単語が後ろに置かれている。
加算の結果だというニュアンスを出したかったので、sumを前にしたが、可読性は変わらなさそう。
- `node* is not None`の処理の重複:ループを使いながらこの重複なしで書こうとすると、
`while node1 is not None and node2 is not None`の後に、
`while node1 is not None`と`while node2 is not None`をぶら下げることになりそう。

n、mをl1, l2のノード数として
- time complexity: O(max(n, m))
- space complexity: O(max(n, m))

# step 2
参考
- https://github.com/philip82148/leetcode-arai60/pull/2 何も知らない人が上から読んでいってパズルができるだけ少ないように書く
- https://github.com/katataku/leetcode/pull/4 入力リストから値を取る関数を用意していた。それによって、値を取る処理とリストを進める処理が別になっていてわかりやすい。sentinelなしでも書けるっぽいがheadの値がループ中で決められており、若干読みにくかった。
- https://github.com/konnysh/arai60/pull/5/files
- 再帰で書いているパターンもあった。dummy_headのメモリ確保が気になるタイプの言語を使っている方に多い印象。
- `if carry == 1:`のチェックもwhileの中に入れている回答があった。
たしかに、「`l1`のn桁目の値(無ければ0)と`l2`のn桁目の値(無ければ0)と繰り上がりの値の和の1の位が
和のn桁目になります。全てが0になった時、加算終了です」の方が、最後の桁チェックだけ別でやるよりも自然。
- step 1では`sum_value = (value1 + value2 + carry) % 10`としていたが、変数名と意味が合っていない。
- https://peps.python.org/pep-0308/ : pythonの三項演算子が導入される過程、なんであの構文になったのかという話。
既存のコードを壊さないように機能を追加するトレードオフを考えながら、どういう構文がいいか・いらないかみたいな話をしていて、まさしくエンジニアリングなのかなぁと思った。

```python3
class Solution:
def addTwoNumbers(self, l1: Optional[ListNode], l2: Optional[ListNode]) -> Optional[ListNode]:
def get_value(node):
if node is not None:
return node.val
return 0

node1 = l1
node2 = l2
Comment on lines +66 to +67
Copy link

Choose a reason for hiding this comment

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

今回のようにノードのvalを順に見ていくだけで変更しないなら、l1,l2を別の変数に移さずに使うのもありだと思います。
Pythonでポインタとかどうなっているのか詳しくないので、間違っていたらすみません🙇‍♂️

carry = 0
dummy_head = ListNode()
node_sum = dummy_head
while node1 is not None or node2 is not None or carry != 0:
value1 = get_value(node1)
value2 = get_value(node2)
value_sum = value1 + value2 + carry

digit = value_sum % 10
carry = value_sum // 10
Comment on lines +76 to +77
Copy link

Choose a reason for hiding this comment

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

divmod というこういう場面でしか使い道のない Built-in function があります。
https://docs.python.org/3/library/functions.html#divmod

node_sum.next = ListNode(val=digit)

node_sum = node_sum.next
if node1 is not None:
node1 = node1.next
if node2 is not None:
node2 = node2.next
return dummy_head.next
```
# step 3
ノードから値を取る処理、次のノードを取る処理は分けられたので分けることにした。

可能ならListNodeをいじって、`node.get_value(default=0)`とかいうメソッドを追加したい。
Copy link

Choose a reason for hiding this comment

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

Python は ListNode を後からいじること自体は可能です。(この投稿の前後を見ておいてください。)
https://discord.com/channels/1084280443945353267/1225849404037009609/1228375055717765190

インスタンスに足すことも可能です。
types.MethodType
https://stackoverflow.com/questions/972/adding-a-method-to-an-existing-object-instance-in-python

ただ、None のときにも欲しいんですよね。
Python は None へのメソッドの追加が確かできなかったと思いますが、それはさておき、たとえば Ruby では相当する物である nil への追加ができますので、できたとしましょう。
そうだとしても None にメソッドを追加されると、これはシングルトンでプログラムのすべての箇所が共用で使っているものです。大規模な開発では大人数で触られると思わぬ事故が起こりそうですね。

Copy link
Owner Author

Choose a reason for hiding this comment

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

コメントありがとうございます。

None にメソッドを追加されると、これはシングルトンでプログラムのすべての箇所が共用で使っているものです。大規模な開発では大人数で触られると思わぬ事故が起こりそうですね。

確かに、気軽にやりたいと思えることではないですね。

一つ伺いたいのですが、「インスタンスにメソッドを足すことが可能」のような知識はどういう流れで身についていくのでしょうか?
リンク先にもある通り、私自身もインスタンスにメソッドを足すはいい慣習ではないと思いますし、今回の問題を解くに当たってやろうともやれるとも思っていませんでした。「メソッドを追加してやろうと思って調べる」なのか、「クラスに関するドキュメントを見ていたら気になったので調べる」なのか、どういうとっかかりでこういう知識がついていくのか気になりました。

Copy link

Choose a reason for hiding this comment

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

Python がどういう動きをしているかから導いています。新しい言語を学ぶときに初期に気になるのがこのあたりです。そして、だいたいマニュアルに書いてあります。

https://discord.com/channels/1084280443945353267/1196472827457589338/1200081854301225011
これも同じかと思います。


コーディング面接でテンパらずにここまで処理をまとめられるようになるのかはよくわからない。
慣れればできるんだろうか。

```python3
class Solution:
def addTwoNumbers(self, l1: Optional[ListNode], l2: Optional[ListNode]) -> Optional[ListNode]:
def get_value(node):
if node is not None:
return node.val
return 0
def get_next(node):
if node is not None:
return node.next
return None

node1 = l1
node2 = l2
carry = 0
dummy_head = ListNode()
node_sum = dummy_head
while node1 is not None or node2 is not None or carry != 0:
value1 = get_value(node1)
value2 = get_value(node2)
value_sum = value1 + value2 + carry

digit = value_sum % 10
carry = value_sum // 10
node_sum.next = ListNode(val=digit)

node_sum = get_next(node_sum)

Choose a reason for hiding this comment

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

node_sum = node_sum.nextでいいような気がしてるんですが、いかがでしょう。
今回のケースでいうとnode_sumがNoneになることはないと思いました。
(get_nextの方が読みやすいでしょうか。好みの範囲?)

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

@thonda28 thonda28 Nov 26, 2024

Choose a reason for hiding this comment

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

僕も node_sum = node_sum.next で十分かなと思いました。(get_value() もかえって複雑なのではと感じました)

また Step1 では「値を読み取ったらすぐに次のノードに動かす」という実装で、Step3 では「値の読み取り」と「ノードの次に進める」という処理を分離する実装になっていますが、Step 1 のほうが着目しているノードに対する処理が1箇所にまとまっていて個人的にはわかりやすいと感じました。(が、これは人によるのかもです)

Copy link

Choose a reason for hiding this comment

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

これは、直前で作っているので None ということはないので、単純に node_sum = node_sum.next でよいのではないでしょうか。

node1 = get_next(node1)
node2 = get_next(node2)
return dummy_head.next
```

# step 4
コメントまとめ
- [divmod](https://docs.python.org/3/library/functions.html#divmod)
- [インスタンスにメソッドを追加する](https://stackoverflow.com/questions/972/adding-a-method-to-an-existing-object-instance-in-python)
- [クラスを後からいじる](https://discord.com/channels/1084280443945353267/1225849404037009609/1228375055717765190)
- ノードを動かすタイミングで二つの意見をもらった
- n桁目の値を取り出したらすぐにノードを進める方が読みやすい
- n桁目の合計を計算する前に、ノードを進めるのは読みにくい

前者はノードに注目をしていて、後者は全体のcomputationに注目をしている気がした。
足し算の各桁での処理は雑な言語化をすれば、
1. n桁目の値を取ってくる。なければ0
2. n-1桁目の繰り上がりと、n桁目の値を足す
3. 加算結果の値から、n桁目の計算結果と、繰り上がりを求める
4. 次の桁にうつる

というような処理をしているので、個人的な好みは後者だった。

```python
class Solution:
def addTwoNumbers(
self,
l1: Optional[ListNode],
l2: Optional[ListNode]
) -> Optional[ListNode]:
def get_value(node):
if node is not None:
return node.val
return 0
def get_next(node):
if node is not None:
return node.next
return None

dummy_head = ListNode()
node = dummy_head
node1 = l1
node2 = l2
carry = 0

while node1 is not None or node2 is not None or carry > 0:
value1 = get_value(node1)
value2 = get_value(node2)
value_sum = value1 + value2 + carry

Choose a reason for hiding this comment

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

こちら、valueにあまり意味がないと思うので、単にsumでもよいと思います。
※valueはListNode.valueということだと思いますが、単にvalueからそれは思いつきにくいと思います。
連続した2行で使われている変数なので、そこまで変数名にこだわる必要はないかと。

ついでにいうと、get_valueやget_nextはnode.next if node else Noneで置き換えられそこまで長くないので僕だったら関数にしません。
でも、それ以外はいいと思いました!

Copy link
Owner Author

Choose a reason for hiding this comment

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

sumだとbuiltin functionと被るんですよね。
ただ、value1, value2自体はそれほど注目しているわけではないので、sum_value = get_value(node1) + get_value(node2) + carryとかでもいいのではと思ってます。


carry, digit = divmod(value_sum, 10)
node.next = ListNode(val=digit)

node = node.next
node1 = get_next(node1)
node2 = get_next(node2)

return dummy_head.next
```

dummy headなし
```python
class Solution:
def addTwoNumbers(self, l1: Optional[ListNode], l2: Optional[ListNode]) -> Optional[ListNode]:
def get_val(node: Optional[ListNode]) -> int:
if node is not None:
return node.val
return 0

def get_next(node: Optional[ListNode]) -> Optional[ListNode]:
if node is not None:
return node.next
return None

value = get_val(l1) + get_val(l2)
carry, digit = divmod(value, 10)
head = ListNode(val=digit)
l1 = get_next(l1)
l2 = get_next(l2)
tail = head
while l1 is not None or l2 is not None or carry > 0:
value = get_val(l1) + get_val(l2) + carry
carry, digit = divmod(value, 10)
tail.next = ListNode(val=digit)
tail = tail.next
l1 = get_next(l1)
l2 = get_next(l2)
return head
```

再帰
```python
class Solution:
def addTwoNumbers(self, l1: Optional[ListNode], l2: Optional[ListNode]) -> Optional[ListNode]:
def get_val(node: Optional[ListNode]) -> int:
if node is not None:
return node.val
return 0
def get_next(node: Optional[ListNode]) -> Optional[ListNode]:
if node is not None:
return node.next
return None

def add_two_numbers_helper(
l1: Optional[ListNode],
l2: Optional[ListNode],
carry: int
) -> Optional[ListNode]:
if l1 is None and l2 is None and carry == 0:
return None

value = carry + get_val(l1) + get_val(l2)
carry, digit = divmod(value, 10)
head = ListNode(val=digit)
head.next = add_two_numbers_helper(
get_next(l1),
get_next(l2),
carry
)
return head
return add_two_numbers_helper(l1, l2, 0)
```