Conversation
|
|
||
| while (carry != 0 || l1 != null || l2 != null) { | ||
| if (l1 != null) { | ||
| carry += l1.val; |
There was a problem hiding this comment.
carryにどんどん値が足されていくのは個人的には少し違和感を感じました。
変数は増えてしまいますがsumなどの変数を定義してそこに足していくほうが分かりやすく感じます。
There was a problem hiding this comment.
たしかにcarryという名前以上の責務を負わせてしまっていますね。
おっしゃる通りwhileループの外で繰り上がりを管理するcarry、ループの中で合計を管理するsumと分けて管理した方が丁寧ですね。
| int l2Val = l2 != null ? l2.val : 0; | ||
| int sum = l1.val + l2Val + carryUp(isCarryUp); | ||
| isCarryUp = false; | ||
| if (sum > 9) { |
There was a problem hiding this comment.
些細な点ですが、マジックナンバーの理解しやすさという観点からsum > 9よりsum >= 10のほうが分かりやすいかもしれません。
Ryotaro25/leetcode_first60#5 (comment)
seal-azarashi/leetcode#5 (comment)
There was a problem hiding this comment.
処理内容を日本語で考えた場合、10以上と記述するのが自然ですね。ありがとうございます！
| } | ||
|
|
||
| private ListNode getNextOrNull(ListNode node) { | ||
| return node == null ? null : node.next; |
There was a problem hiding this comment.
このくらいの短さならいいと思うのですが、三項演算子はわかりにくいので、このコードはあまり好みじゃないです。
There was a problem hiding this comment.
コメントありがとうございます！
個人的にはワンライナーで書けるこの程度までは三項演算子で書いてしまいます。
とはいえ三項演算子を読みづらいという方も結構いることを心に留めておきます。
私もネストされた三項演算子とか読みづらいです。
| private int getValOrDefault(ListNode node) { | ||
| return node != null ? node.val : 0; | ||
| } |
There was a problem hiding this comment.
（getNextOrNullもですが）
処理内容がメソッド名になっていて、メソッド名の長さと処理内容の長さがほぼ変わらないので個人的にメソッド化するほどでもないかなと思いました。
自分がこれくらい短い処理にメソッド名をつけるとしたら、処理内容の意図をメソッドに反映させたい時ですかね。
There was a problem hiding this comment.
carry = carry + getValOrDefault(l1) + getValOrDefault(l2) ;
と書けるのがちょっと気持ちいいくらいの効果しか無いですね。
自分がこれくらい短い処理にメソッド名をつけるとしたら、処理内容の意図をメソッドに反映させたい時ですかね。
処理に名前を付けて意図を明示するということですね。その基準を意識してみます。
| class Solution { | ||
| public ListNode addTwoNumbers(ListNode l1, ListNode l2) { | ||
| ListNode dummy = new ListNode(); | ||
| ListNode node = dummy; |
There was a problem hiding this comment.
細かいですが、個人的にはdummyHeadなど何のダミーなのかが表現されている方がわかりやすく感じます。
4 participants
今回解いた問題：
2. Add Two Numbers
次に解く問題：
20. Valid Parentheses