Conversation
| carry = value_sum // 10 | ||
| node_sum.next = ListNode(val=digit) | ||
|
|
||
| node_sum = get_next(node_sum) |
There was a problem hiding this comment.
node_sum = node_sum.nextでいいような気がしてるんですが、いかがでしょう。
今回のケースでいうとnode_sumがNoneになることはないと思いました。
（get_nextの方が読みやすいでしょうか。好みの範囲？）
There was a problem hiding this comment.
コメントありがとうございます。
やっていることは変わらないので、好みの問題な気がしますが気持ちとしては、
- ノードを一つ進める処理がまとまっていて、かつ、おんなじ書き方している方が読みやすそう。（なんで一個だけ違う書き方してるんだと、一瞬考える時間がなくなる）
- この関数呼び出しがボトルネックにはならなさそう
くらいの考えでした。
There was a problem hiding this comment.
僕も node_sum = node_sum.next で十分かなと思いました。（get_value() もかえって複雑なのではと感じました）
また Step1 では「値を読み取ったらすぐに次のノードに動かす」という実装で、Step3 では「値の読み取り」と「ノードの次に進める」という処理を分離する実装になっていますが、Step 1 のほうが着目しているノードに対する処理が1箇所にまとまっていて個人的にはわかりやすいと感じました。（が、これは人によるのかもです）
| # step 3 | ||
| ノードから値を取る処理、次のノードを取る処理は分けられたので分けることにした。 | ||
|
|
||
| 可能ならListNodeをいじって、`node.get_value(default=0)`とかいうメソッドを追加したい。 |
There was a problem hiding this comment.
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 にメソッドを追加されると、これはシングルトンでプログラムのすべての箇所が共用で使っているものです。大規模な開発では大人数で触られると思わぬ事故が起こりそうですね。
There was a problem hiding this comment.
コメントありがとうございます。
None にメソッドを追加されると、これはシングルトンでプログラムのすべての箇所が共用で使っているものです。大規模な開発では大人数で触られると思わぬ事故が起こりそうですね。
確かに、気軽にやりたいと思えることではないですね。
一つ伺いたいのですが、「インスタンスにメソッドを足すことが可能」のような知識はどういう流れで身についていくのでしょうか？
リンク先にもある通り、私自身もインスタンスにメソッドを足すはいい慣習ではないと思いますし、今回の問題を解くに当たってやろうともやれるとも思っていませんでした。「メソッドを追加してやろうと思って調べる」なのか、「クラスに関するドキュメントを見ていたら気になったので調べる」なのか、どういうとっかかりでこういう知識がついていくのか気になりました。
There was a problem hiding this comment.
Python がどういう動きをしているかから導いています。新しい言語を学ぶときに初期に気になるのがこのあたりです。そして、だいたいマニュアルに書いてあります。
https://discord.com/channels/1084280443945353267/1196472827457589338/1200081854301225011
これも同じかと思います。
| carry = value_sum // 10 | ||
| node_sum.next = ListNode(val=digit) | ||
|
|
||
| node_sum = get_next(node_sum) |
There was a problem hiding this comment.
これは、直前で作っているので None ということはないので、単純に node_sum = node_sum.next でよいのではないでしょうか。
| digit = value_sum % 10 | ||
| carry = value_sum // 10 |
There was a problem hiding this comment.
divmod というこういう場面でしか使い道のない Built-in function があります。
https://docs.python.org/3/library/functions.html#divmod
| node1 = node1.next | ||
| if node2 is not None: | ||
| value2 = node2.val | ||
| node2 = node2.next |
There was a problem hiding this comment.
この部分がわかりづらく思いました。所与の連結リストを走査していき、ノードがあるならvalue1ないしはvalue2にその数字を格納して、ノードがすでに終わっているなら加算するものがないから0を代入する、というロジックかと思います。
if node1 is not None:
value1 = node1.val
else:
value1 = 0が一番しっくりくると思います。
また、合計を計算したりする前にノードを次に進めているのもわかりづらく感じました。
| 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 |
There was a problem hiding this comment.
value1, value2 はともに一時変数だと思うので carry も含めて1つの変数で扱うのがわかりやすいのでは、と思いました。結局興味があるのは value1 + value2 + carry なので。（tmp_val がよい変数名かは微妙な気もしますが）
| 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) |
| if carry == 1: | ||
| sum_node.next = ListNode(val=1) |
There was a problem hiding this comment.
この問題において carry には 0 または 1 しか入りませんが、ここで気にしていることは carry があるかないか（0 かそうでないか）かと思うので、if carry: または if carry > 0: という条件のほうが意図が伝わる気がします。
またこの問題では2つのノードですが、後からやっぱり3つのノードも扱えるよう拡張したりすると、（新たに2が入りうるため）この部分で予期せぬバグが生まれたりもしそうです。（実務だとそういった後からの拡張もありえそうです）
| carry = value_sum // 10 | ||
| node_sum.next = ListNode(val=digit) | ||
|
|
||
| node_sum = get_next(node_sum) |
There was a problem hiding this comment.
僕も node_sum = node_sum.next で十分かなと思いました。（get_value() もかえって複雑なのではと感じました）
また Step1 では「値を読み取ったらすぐに次のノードに動かす」という実装で、Step3 では「値の読み取り」と「ノードの次に進める」という処理を分離する実装になっていますが、Step 1 のほうが着目しているノードに対する処理が1箇所にまとまっていて個人的にはわかりやすいと感じました。（が、これは人によるのかもです）
| node1 = l1 | ||
| node2 = l2 |
There was a problem hiding this comment.
今回のようにノードのvalを順に見ていくだけで変更しないなら、l1,l2を別の変数に移さずに使うのもありだと思います。
Pythonでポインタとかどうなっているのか詳しくないので、間違っていたらすみません🙇♂️
| 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 |
There was a problem hiding this comment.
こちら、valueにあまり意味がないと思うので、単にsumでもよいと思います。
※valueはListNode.valueということだと思いますが、単にvalueからそれは思いつきにくいと思います。
連続した2行で使われている変数なので、そこまで変数名にこだわる必要はないかと。
ついでにいうと、get_valueやget_nextはnode.next if node else Noneで置き換えられそこまで長くないので僕だったら関数にしません。
でも、それ以外はいいと思いました！
There was a problem hiding this comment.
sumだとbuiltin functionと被るんですよね。
ただ、value1, value2自体はそれほど注目しているわけではないので、sum_value = get_value(node1) + get_value(node2) + carryとかでもいいのではと思ってます。
7 participants
問題
https://leetcode.com/problems/add-two-numbers/description/
次に解く問題
https://leetcode.com/problems/valid-parentheses/description/