Conversation
|
ファイルをコミットできてなさそうです。 |
|
失礼しました！コミットし直しました。ご指摘ありがとうございます。 |
| func addTwoNumbers(l1 *ListNode, l2 *ListNode) *ListNode { | ||
| dummy := &ListNode{Val: -1, Next: nil} | ||
| added := dummy | ||
| carryUp := false |
| carryUp := false | ||
|
|
||
| for l1 != nil || l2 != nil || carryUp { | ||
| s := 0 |
| s := 0 | ||
| if l1 != nil { | ||
| s += l1.Val | ||
| l1 = l1.Next |
There was a problem hiding this comment.
足すのとノードを動かすのを一緒にやるか、分けるかは好みですかね？自分は足す処理は足す処理で書いて、ノードを動かすのは別に書くのが好みです。
| l2 = l2.Next | ||
| } | ||
|
|
||
| if carryUp { |
There was a problem hiding this comment.
自分はヘルパー関数とかを用意してあげて以下のように書く気がします。carryは0, 1で持っている。
sum = getValue(l1) + getValue(l2) + carryThere was a problem hiding this comment.
たしかにそう書いたらもっとすっきりしそうですね。ありがとうございます
|
|
||
| ```Go | ||
| func addTwoNumbers(l1 *ListNode, l2 *ListNode) *ListNode { | ||
| dummy := &ListNode{Val: -1, Next: nil} |
There was a problem hiding this comment.
sentinelという単語は外ではあまり見かけずこの界隈ではよく見るのですが、「ソフトウェアエンジニアの常識」に入りますか？
There was a problem hiding this comment.
自分は常識に入っていると思います。
日本語だと「番兵」と呼ばれることが多いように思います。
https://ja.wikipedia.org/wiki/%E7%95%AA%E5%85%B5
https://en.wikipedia.org/wiki/Sentinel_node
https://en.wikipedia.org/wiki/Sentinel_value
X (Twitter) でも聞いてみましょう…。
https://x.com/nodchip/status/1815199895860707526
There was a problem hiding this comment.
https://source.chromium.org/search?q=%22sentinel%22
「番兵」という概念はときどき使います。変数名として使うのがとても一般的とまでは思いません。
There was a problem hiding this comment.
番兵の使用例。
https://discord.com/channels/1084280443945353267/1201211204547383386/1241785484460556308
https://discord.com/channels/1084280443945353267/1235829049511903273/1246118347863621652
https://discord.com/channels/1084280443945353267/1252267683731345438/1252556045524537344
https://discord.com/channels/1084280443945353267/1201211204547383386/1253026284938858536
There was a problem hiding this comment.
https://x.com/nodchip/status/1815199895860707526
プログラミング用語の「番兵」という言葉を知っている人と知らない人が、ほぼ半々のようですね。ソフトウェアエンジニアの常識には含まれていなさそうです。
チームで開発する際、チームの平均的なエンジニアが sentinel という単語を知っていて、変数名に使っても違和感を感じないのであれば、使っても良いと思います。
There was a problem hiding this comment.
わざわざ集計ありがとうございます。少なくとも協会参加者内だと常識ということでここでは使っていこうと思います。
| l1, l2 = l1.Next, l2.Next | ||
| } | ||
|
|
||
| for l1 != nil { |
| carryUpFlag := false | ||
|
|
||
| for l1 != nil || l2 != nil || carryUpFlag { | ||
| s := 0 |
There was a problem hiding this comment.
変数のスコープの長さがそこそこありますので、変数の値が表すものを端的に表した英単語・英語句を変数名として付けることをお勧めいたします。
| func addTwoNumbers(l1 *ListNode, l2 *ListNode) *ListNode { | ||
| dummy := &ListNode{Val: -1, Next: nil} | ||
| addedNode := dummy | ||
| carryUpFlag := false |
There was a problem hiding this comment.
flag という単語にはほとんど情報量がないため、 step 3 のように carryUp としたほうが良いと思います。
2AddTwoNumbers.md
Outdated
| - addedNodeをaddedに変更。`added := dummy`から、`added`がノードであることを推測できるから。 | ||
| - carryUpFlagをcarryUpに変更。`if carryUpFlag`より`if carryUp`の方が自然だと思った。 | ||
|
|
||
| ``` |
| return dummy.Next | ||
| } | ||
|
|
||
| func addTwoNodes(addedNode, l1, l2 *ListNode, carryUpFlag bool) { |
There was a problem hiding this comment.
step3でコメントありましたが、carryは自然数で持つほうが自然かなと思います
今回は問題設定の性質上carryが0 or 1 ですが、場合によっては2とかもありうるので
| for l1 != nil || l2 != nil || cu == do { | ||
| sum := getVal(l1) + getVal(l2) + int(cu) | ||
| cu = dont | ||
| if sum >= 10 { |
There was a problem hiding this comment.
carryUpは数値で持たせsum / 10で算出すると、この分岐を消せてコードをシンプルにできます。
| getVal := func(node *ListNode) int { | ||
| if node == nil { | ||
| return 0 | ||
| } | ||
| return node.Val | ||
| } | ||
|
|
||
| moveNode := func(node *ListNode) *ListNode { | ||
| if node == nil { | ||
| return node | ||
| } | ||
| return node.Next | ||
| } |
There was a problem hiding this comment.
わざわざメソッドにするほどの処理でもなさそうに思います。
また、メソッド名から受け取る印象と実際の挙動の差分に違和感を感じました。
getValという名前で0が返ってきた時、それはnilを渡したからなのかnodeのvalが実際に0だからなのか区別がつかず、混読みづらさを感じます。
moveNodeについても同じで、moveしてるはずなのに渡したノードそのものが返ってくるのは読んだ時に違和感を覚えます。
6 participants
https://leetcode.com/problems/add-two-numbers/description/